Check results are now reported in the status checks at the bottom of this page.

It's also worth noting, that there is only one other use of make_function_with_signature in astropy, in modeling. Which could probably be replaced with direct manipulation of the Python 3 signature machinery. So we could probably remove all of that as well.

It appears there is one quirk with supports_nddata which I don't understand, but I am not going to waste my time debugging it until people are actually on board with this plan.

Ok so just to be clear, wraps in Python 3.5 now no longer requires us to have our own implementation? If so, that's fine by me in principle

@astrofrog Yes, but not quite with perfect feature parity.

I am not going to waste my time debugging it

Looks like a "catch 22" situation. I am not convinced until tests can pass. 😉

haha, the question is, are we willing to loose the exclude_args and name trickery for the sake of this refactor? If so then I will try and make it work. Obviously if I can't make it work it's just a waste of everyones time...

I cannot comment on the nddata stuff but maybe @mwcraig or @MSeifert04 can?

I’m gong to be offline for the next week, can look when I’m back

Very much in favour from the peanut gallery! Though since it is actually in the documentation, I think wraps also would need to be deprecated first (as far as I'm concerned, this can be by returning the python standard version with a deprecation warning that it will not stay in astropy.utils.decorators).

I didn't find a single place on github where this was used, so we most probably can get away with @mhvk's suggestion for a very quick deprecation period.

@bsipocz did you also check for make_function_with_signature by chance? I assume we should deprecate that for a little while as well?

@Cadair - There is minimal use for that, and most of the hits seems to be in copies of astropy itself: https://github.com/search?q=make_function_with_signature&type=Code

I've only found one genuine use of it in one of Perry's repos, so my guess is the usage is indeed fairly low.

I like it - provided we don't completely break support_nddata :)

Too controversial to merge so close to freeze time, so re-milestoned. 😬

I think this is one for the meeting in Dec.

meeting in Dec

OK, added to breakout planning board.

OK, added "keep open". I think there was general consensus that this was a good idea as long as support_nddata keeps working.

@Cadair - Could you please rather rebase this that keeping it alive by the label? IMO we shouldn't over use the label otherwise easily end up it the previous situation of having long abandoned PRs all over.

@bsipocz - that was the fault of me and @pllim - but point taken that a rebase would be better.

Codecov Report

Merging #7554 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #7554      +/-   ##
==========================================
+ Coverage    86.9%   86.93%   +0.02%     
==========================================
  Files         393      397       +4     
  Lines       58584    59123     +539     
  Branches     1064     1100      +36     
==========================================
+ Hits        50914    51397     +483     
- Misses       7051     7085      +34     
- Partials      619      641      +22

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d1f2fd5...387cab3. Read the comment docs.

Waiting to see if doc issue still occurs. It does not locally (though I'm seeing an error in an unrelated part of the docs)

Doc still failed. FYI

Warning, treated as error:
toctree references unknown document 'api/astropy.utils.decorators.wraps'
Sphinx Documentation subprocess failed with return code 2
Exited with code 2

Doc still failed. FYI

Yeah, still can't reproduce locally, which is making it hard to troubleshoot....

OK, reproduced and fixed (hopefully). The other changes in the last push were to replace imports of astropy.utils.wraps with functools.wraps so that we can eventually not provide wraps at all in astropy.utils.

Awesome!! Thanks @mwcraig

Since the deprecation from this PR has been reverted in #8624, I'm adding the no-changelog-entry-needed label to stop the release script from complaining.

For the sake of the exercise I'm looking at recent benchmark regressions and weirdly this came up as a reason for a 1.3-1.5x slowdown in the convolution benchmarks.
http://www.astropy.org/astropy-benchmarks/#regressions?sort=1&dir=desc

Hmm... Do you think this is because it is using modeling and modeling is using the deprecated code?

Yes, I think it's modeling related, but the regression should have show up in the deprecating PR if it's because of that. I suspect just there are some overheads here that we didn't think about, and/or some non optimal usage of modeling in convolution.

I would say that we need to keep an eye on those benchmarks after the refactoring PR is merged in. Or, do you @astrofrog think that we can run the benchmarks on the PR to check the performance implications? (though either way the refactoring will be merge, so maybe it's not urgent to check it in the PR stage).