Improve SciPy methods deprecation process

[AnirudhDagar]

Recently, while working on #15901 I realized certain problems and issues with our deprecation process. The following tasks should help improve the overall experience for the developers as well as the users reading the docs.

1. Need for deprecation guidelines: Currently, there is no definitive documented process for devs to follow when deprecating functionality (other than this small generic section on deprecation) in SciPy. I see some deprecations using @np.deprecate decorator and some from scipy._lib.deprecation import _deprecate and then decorating with @_deprecate. Then within the docstring, some methods use .. deprecated:: sphinx directive while others do not. I strongly suggest that one must use the deprecated sphinx directive in the docstring everytime we make a deprecation. Just adding the decorator for deprecation is not enough for the docs to look good (highlight the deprecation in a red box). Also some functions (eg kulsinki) after getting deprecated are hidden from the documentation toctree before the deprecation cycle ends for that method which is not ideal and we should avoid/fix that. All of this should be clearly mentioned for the devs to follow.

2. Failing docstring example after deprecation: After a method is wrapped with a deprecation warning, and the deprecation cycle starts, now the docstring examples for the functions fail (as discussed in the PR #15901) due to the deprecation warning turned into an error by matplotlibs plot directive. We should have a standard way to skip those deprecation warning errors in conf.py config for matplotlib sphinx plot directive. Maybe there is way to automatically ignore the dep warning for this in doctests as soon as it detects a .. deprecated:: sphinx directive in the doc. Needs more discussion as to what is the best practice here. (PR DEP, DOC: Show deprecated methods in docs and fix overwriting with _deprecated #16866)

3. Dep message repetition or completely overwritten docs: Remove repetition of deprecation message when using sphinx directive as well as the scipy._lib.deprecation._deprecate decorator. Docs should only show the deprecation warning with the message in a special red box, not plain text, coming from the sphinx directive. Atm the warning is repeated when np.deprecate is used and the docs are completely overwritten when scipy._lib.deprecation._deprecate is used . See example of this repetition issue here and overwritten issue here . This is easy to fix by always using scipy's internal scipy._lib.deprecation._deprecate decorator and making a little tweak like this (will add this tweak when sending a PR for the dep guidelines). (PR DEP, DOC: Show deprecated methods in docs and fix overwriting with _deprecated #16866)

4. Clickability of links: In the deprecation box made from .. deprecated:: sphinx links do not work. The fix needs to be made in paydata-sphinx-theme. Again check the same example illustrating the issue in scipy.misc.face here. (PR BUG: Fix links not clickable in versionmodified directives pydata/pydata-sphinx-theme#877)

All of the above was discussed offline on SciPy slack, I thought it would be better to summarise everything with proper links in this issue.

cc @tupui @j-bowhay @rgommers

[tupui]

Thanks for the write up!

We have a bit of documentation around deprecating something IIRC. We should indeed expand it to include some concrete guidelines.

My 2 cents. I am not convinced about the need of any decorator as it's mostly just a warning to raise. So we really don't spare much writing. But maybe that can help with detecting things for the doctests skipping.

[j-bowhay]

My 2 cents. I am not convinced about the need of any decorator as it's mostly just a warning to raise.

I think this is a very valid point. As well as @np.deprecate & from scipy._lib.deprecation import _deprecate you can just write the warning which is fairly trivial to do:

warning.warn("A is deprecated in favour of B since SciPy 1.10.0 and will be removed in SciPy 1.12.0",
             DeprecationWarning, stacklevel=2)

It is worth adding that if my memory is correct the scipy _deprecate decorator is the only one of these 3 that causes the refguide check to fail as seen in #16273 (although obviously if you then use the deprecated function in an example this will fail).

[AnirudhDagar]

My 2 cents. I am not convinced about the need for any decorator as it's mostly just a warning to raise. So we really don't spare much writing. But maybe that can help with detecting things for the doctests skipping.

@tupui Yes, it could be used to automatically detect doctest skipping that's what I'm thinking, even if it is not used, I feel it is a good practice to use decorators in such cases for much cleaner code and keep the method logic separate from the deprecation logic (even though a one-two liner). But that's just my opinion on code style :P, no right wrong.

It is worth adding that if my memory is correct the scipy _deprecate decorator is the only one of these 3 that causes the refguide check to fail as seen in #16273 (although obviously if you then use the deprecated function in an example this will fail).

@j-bowhay atm in fact using _deprecate is the only one where refguide passes because it is overwriting the docs completely replacing the docs with plain text warning, ike in binom_test. But after the fix that I'll make, the overwriting should go away and should behave similarly to np.deprecate, except that warning will be printed only once through the sphinx directive, while np.deprecate causes a repeat.

refguide checks only fail when there is an example of the function in the docstring (which is generally there to showcase that method use). This will need to be skipped/ignored in the doctests, as mentioned above.

[j-bowhay]

To me it looks like #16866 dealt with the second point too so it looks like just better guidelines left.

[AnirudhDagar]

Yes, that is true @j-bowhay. After thinking about point one, I was not sure what exactly we should add to the deprecation guidelines since we use both the numpy decorator as well SciPy's own deprecation decorator. Any suggestions would be welcome :)

[j-bowhay]

I think I've came to a slightly more liberal point of view on this than I previously had: there are 3 ways of adding the warning but ultimately if the user sees the warning correctly I am not sure it matters! I don't think there is any point getting super picky about code that is created to be deleted. I think the documentation should focus on the higher level process:

1. Is the deprecation necessary / worth the inconvenience to the user
2. Getting mailing list approval
3. Adding a warning that the user will see when they run their code
4. Adding a warning in the documentation stating what is deprecated
5. Ensure that both these warning messages have the following:
   • What is deprecated
   • What version it was deprecated in
   • What version behaviour will change in
   • What the user should do instead
6. Ensuring there is a test which checks the warning is raised
7. Deprecation is added to the release notes and META issue

What do you think?