Handle deprecation in a standard and consistent way?

[jimmylai]

We have a couple things deprecated and pending removal in the future.

Can we have a standard and consistent methodology to handle deprecation and removal?

What we have now:

• BaseAssignment.accesses calls warnings.warn(..., DeprecationWarning) when it's been called.
• BatchableMetadataProvider, MetadataWrapper, and other metadata related classes were moved to libcst.metadata but there were still available in libcst for backward compatibility.
• ExtSlice is deprecated and we have inline comments in the code.
• SyntacticPositionProvider and BasicPositionProvider are about to be deprecated in Rename position provider classes #114
• TODO: move BaseMetadataProvider to libcst.metadata

The warnings.warn(..., DeprecationWarning) seems to be a widely adopted way to warn on deprecation. Can we try to use it on all cases? For class deprecation, we don't want to just subclass since that may require lots of other changes to make existing callsite/type-checking work. Can we somehow have a helper to wrap all all methods in a class to call warnings.warn(..., DeprecationWarning)?

There are more things we can do, e.g. add the deprecation warning to readthedoc docs, so reader see it. Or explicitly declare the target version that deprecated things will be removed.
There are deprecation packages provides a @deprecation to make all those easier:

• https://deprecation.readthedocs.io/en/latest/
• https://pypi.org/project/Deprecated/

Some projects put all deprecated things together in a doc for tracking, e.g.
https://django.readthedocs.io/en/1.5.x/internals/deprecation.html
numpy/numpy#11521
This is more like a nice to have to me.

[DragonMinded]

Any sort of explicit warning we can give to developers well in advance of removal is good. I am not sure I have any opinions on warnings.warn(..., DeprecationWarning) versus @deprecated. They both seem to have their advantages and assumptions.

I think you are right that we need some semi-formal deprecation documentation. I was proposing to @bgw that we have a document at the root called DEPRECATION which lists our policy and things slated for deprecation as well as recommended alternatives. Some day in the future, we can also list a command to run which uses LibCST codemod capabilities to auto-upgrade code, but that's not currently possible. I like the idea of having a deprecation section in the readthedocs, because its a perfect place to list each individual deprecation as well as example code and links to the docs for what to use instead. I would assume we can do this in addition to a DEPRECATION file or similar. Maybe we want to just update the README with a Deprecation section close to the Future section?

[bgw]

I've seen a lot of projects use their changelog (and by extension, the github releases page) for declaring deprecations.

[DragonMinded]

Ooh, yeah that's not a bad place to put things as well. Especially once we actually drop support for old features.

[carljm]

To the extent there's a "standard approach," it's to document things in changelog / release notes (as well as the reference documentation for the affected items), and then if possible also warnings.warn(DeprecationWarning) at runtime.

The deprecation library providing the @deprecated decorator I hadn't seen before, seems like just a wrapping around warnings with some bells and whistles.

[jimmylai]

Circle back on this, we already have to many things deprecated but the deprecation process wasn't standardized. It looks messy to me.
Can we start using the deprecation library to add deprecated_in, removed _in and fail_if_not_removed in the code? The library wrapped Python warnings helper and automatically update the docstring (so the deprecation versions show up in Sphinx doc). It also automatically fails tests when we upgrade to target version but forget to remove the deprecated functions.

[DragonMinded]

What are we solving here? We have a deprecation strategy: Things get deprecated on a minor release bump (such as 0.2.x -> 0.3.0) which has not happened yet. Adding a library won't help that, and we already have a record of deprecations in the changelog. Many of the things deprecated can't be decorated, and can only be documented with docstrings. Its unclear what adding another layer does for us in this case.

[jimmylai]

Using the library is to make the deprecation documentation easier.
Current the deprecated things weren't explicitly say they'll be removed in the version of 0.3.0.
The deprecation library can help us add an explicitly removal version in documents for user and raised exception when we already upgrade to new version but didn't remove them.
We can also do that manually but that's more repetitive.

[jimmylai]

The way we handle it currently is adding deprecated in comment and documentation.
Then clean them up in the next major version release.
It's simple and works ok since we don't have many to deprecate.