convolve_fft defaults and documentation

[astromark]

I've just been trying to run some code that I haven't used for a year or two, when I had an older version of astropy installed (I'm now running 2.0.10). It was giving very strange results and I eventually managed to track this done to convolve_fft. I found that if I change my convolve_fft(image,psf) to convolve_fft(image,psf,nan_treatment='fill',normalize_kernel=False) I could then reproduce the results I was getting previously. I take it the defaults were changed at some point, presumably related to #926. Can I suggest that it would make debugging old code much easier if changes that break backwards compatibility, such as this, are noted in the documentation? I'm thinking of something along the lines of http://docs.astropy.org/en/v2.0.x/io/fits/api/files.html#writeto where it clearly states 'Changed in version 1.3: overwrite replaces the deprecated clobber argument.'

The examples also need to be completely rewritten as they were clearly written using the old defaults. For instance, the second one convolve_fft([1, np.nan, 3], [1, 1, 1]) now gives the result array([0.5, 2. , 1.5]).

[bsipocz]

Were do you find that example that is giving a different result than listed for the example? We normally test those automatically, so if they are wrong then something must have got broken along the way.

[astromark]

Here for example:
http://docs.astropy.org/en/stable/api/astropy.convolution.convolve_fft.html

[bsipocz]

You're right. We somehow have a __doctest_skip__ = ['*'] in that file, and that shouldn't be the case. Thanks for reporting.

[keflavich]

Right, we had major backward compatibility breaking changes in the v2.0 release, I believe. Those were (should have been?) documented in the changelog, but not directly in the docs.

[astromark]

Another place that I've noticed an issue is the 'What's new in Astropy 2.0?' document, which specifically states "convolve_fft’s behavior remains unchanged".
http://docs.astropy.org/en/stable/whatsnew/2.0.html#improvements-to-astropy-convolution

[bsipocz]

There were significant convolution changes in 3.1, too, they might have been internal only though.

[lupitatovar]

Hello, this is my first time contributing to Astropy and I would like to try and resolve this issue if that's alright?

[geekypathak21]

I want to work on this issue __doctest_skip__ = ['*'] needed to be removed or not

Right, we had major backward compatibility breaking changes in the v2.0 release, I believe. Those were (should have been?) documented in the changelog, but not directly in the docs.

As, mentioned here they should be in changelog

[bsipocz]

yes, the doctests shouldn't be skipped, but in order to remove __doctest_skip__ they need to be fixed, too.

[astrojuanlu]

Prospective contributors: #8510 was an attempt to fix the doctests, but it was incomplete because __doctest_skip__ needs to be removed as well. The offending file is astropy/convolution/convolve.py.

[aaryapatil]

I am running a "Contributing to Astropy" workshop at the Pan-African School for Emerging Astronomers (Astropy funded project). I will be guiding the students through one or two "first contributor" issues. I'm wondering if this issue is still relevant, and is the only thing necessary not skipping doctests and ensuring the doctests are fixed? If there is more context available here, it would be great for new students!

[pllim]

astropy/convolution/convolve.py does not skip doctest anymore, so maybe this can be closed without further action?

[aaryapatil]

Ah, I'll find another one then. There don't seem to be many first contributor issues...

[astromark]

As the person who originally reported this issue, I should probably chime in here. It's great that the doctests were fixed. Still, I wonder if there could still be some improvements to the documentation. For instance, it doesn't really make sense for there to be examples with keywords that are the defaults anyway since these all show the same result e.g.:

convolve_fft([1, np.nan, 3], [1, 1, 1])
convolve_fft([1, np.nan, 3], [1, 1, 1], nan_treatment='interpolate')
convolve_fft([1, np.nan, 3], [1, 1, 1], nan_treatment='interpolate', normalize_kernel=True)

all give the same result and so are redundant. Examples with the previous defaults of nan_treatment='fill' and normalize_kernel=False would be more useful.