Decorate deprecated command options

[mih]

Right now, we do this by editing the docstring and adding custom migration code and issue DeprecationWarning when that code is triggered. Example:

        dryrun=Parameter(
            args=("--dryrun",),
            action="store_true",
            doc="""Deprecated. Use the renamed ``dry_run||--dry-run``
            parameter"""),

This makes the fact that an option should no longer be used inaccessible programatically. What about an explicit decoration instead. Maybe like:

        dryrun=Parameter(
            args=("--dryrun",),
            action="store_true",
            doc="This could still be here, but could also be left out completely",
            deprecated="Use the renamed ``dry_run||--dry-run`` parameter")

With such an annotation, a GUI could decide to not even bother showing deprecated commands anymore, rather than having to handling deprecated ones on top of the new interface.

[yarikoptic]

Related: #3496 . I support such centralized/formalized annotation. Moreover I would have added a version there since it would be known since what version it would be deprecated (0.MINOR+1.0) when that PR to be merged into master, so I would have also added explicit deprecated_version option (so it could be formatted nicely/as needed in rst/docs etc).

[mih]

So the target development would be the support of deprecated_version=. With that, a deprecation message can be autogenerated. The doc= setting could then be used to contain additional information on transitions or mitigation, that I orginally foresaw for deprecated.

[adswa]

I started to tackle this, and for now stopped because I found myself adapting to a lot of the complexities our doc-building system has (and thus adding more) , and because I felt that I couldn't anticipate what would be the best approach to fit both the state of things in datalad-core and the future in datalad-next (e.g., parameter validation). What I have identified is the following:

• We either deprecate entire commands (e.g., uninstall) or individual command parameters (e.g., dry_run). Both of those need a place for annotation, ideally outside of the command classes _params_ as _params_ wouldn't map to full command deprecations. I thought about class attributes, e.g., _deprecated_ and _deprecated_params_.

diff --git a/datalad/distribution/uninstall.py b/datalad/distribution/uninstall.py
index 06914b5b8..7fc9cb9a2 100644
--- a/datalad/distribution/uninstall.py
+++ b/datalad/distribution/uninstall.py

 @build_doc
 class Uninstall(Interface):
 """DEPRECATED: use the `drop` command"""
     _action = 'uninstall'
 
     _params_ = dict(
         [...]
         check=check_argument,
         if_dirty=if_dirty_opt,
     )
+    _deprecated_ = """The `uninstall` command is deprecated and will be removed 
+    in a future release. Use the `drop` command for safer operation instead."""
 
     @staticmethod
     @datasetmethod(name=_action)

• A deprecation warning needs at least a warning message. This can be used to issue a proper Python warnings.warn DeprecationWarning, and to add to the docstrings in appropriate places. It could get an annotation about planned removals as well (as done in e.g., https://deprecation.readthedocs.io/en/latest/), but we so far haven't been very stringent with that
• The deprecation should make its way into the rendered docs in the Python and Command line API in a standardized way (e.g., prefixed docstring for entirely deprecated commands). This requires support in the set of functions that build docstrings, and the set of functions that generates the command line help. I'm unsure how to approach this: The current approach (adding the deprecation notices right into docstrings and parameter descriptions) is clearly the least complex code-wise, but prevents centralized annotations. But centralized annotations need to add helper functions for the Python and CMD docbuilds (when I tried this, I had to add two and wasn't happy with that)
• datalad.cli.interface.py defines a _deprecated_commands dictionary, and it is used to hint at the deprecation and also suggest alternatives when used. Maybe a deprecation annotation could centralize this functionality as well.

[yarikoptic]

why not

 @build_doc
 @deprecated(version='0.18.2', msg='Use the `drop` command for safer operation instead.')
 class Uninstall(Interface):

which would do needed changes to docstring etc?

[adswa]

datalad-next now has a decorator that can be used to emit automatic Deprecation Warnings: datalad/datalad-next#320. We can port it over to core if there is demand :)

[bpoldrack]

We can port it over to core if there is demand :)

Yes, please.

[yarikoptic]

awesome! Any relation/comparison to those existing https://deprecation.readthedocs.io/en/latest/ https://pypi.org/project/Deprecated/ we had discussed in the past (#3496) ?

[adswa]

Yes, see also the discussion in the linked PR. The difference to the deprecated Python package is the ability to annotate command arguments (e.g., --dry-run) and command argument options (e.g., --mode export).

Patching of docs wasn't done because it would require two separate annotations: One one the class level (because only there we can hook into the build_doc system), and one at the e.g. __call__ method of a class, because only there we can evaluate whether anything deprecated (argument, option) has actually been used by a user. So docstrings still require manual attention. I suppose that a deprecation table could be created by simply parsing decorators, but that I haven't done or thought closely about.

[adswa]

Yes, please.

Just a heads up that I will be on vacation or otherwise busy for the next two weeks