Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-557

Deprecation procedure for methods and classes

    Details

    • Type: RFC
    • Status: Implemented
    • Resolution: Done
    • Component/s: DM
    • Labels:
      None

      Description

      RFC-213 was adopted, approving the idea of a standard deprecation procedure, but no specific procedure has been developed.  This RFC attempts to resolve that omission.

      Overall, I propose that when a feature/class/function is deprecated:

      • A description of the deprecation is included in the release notes for the next official release (major or minor or bugfix).  It may be desirable to repeat that description in succeeding release notes until the code is removed. That removal occurs, at the earliest, immediately after the next major release following the release with the first deprecation release note; at the latest, immediately before the major release following.  In other words, if deprecation is first noted in release 17.2.3, the code cannot be removed until after 18.0 is released and must be removed before 19.0 is released.  In general, no deprecation should be removed before two calendar months have elapsed.
      • CI tests are run with deprecation warnings enabled (perhaps using python -Wd).  Triggering such a warning does not cause a test failure.
      • Deprecations are changes to interfaces.  (If they were limited to implementation, they wouldn't require deprecation.)  As a result, they often require or result from RFCs.  But our usual policy applies; if no one would object, a deprecation can be made without an RFC.

      I propose that that deprecation be handled as follows for Python functions and classes:

      • We add the deprecated package and its dependencies to the conda environment. More on why this package was selected below.
      • Code that contains deprecated classes or functions does "from deprecated.sphinx import deprecated".
      • @deprecated(reason="why") is used to decorate any class or function that is to be deprecated. The reason string should include the replacement API when available or explain why there is no replacement. Since we generally will not be adding deprecation decorators in advance of when the deprecation actually begins, the version argument is unnecessary.
      • Since our end users tend to be developers or at least may call APIs directly from notebooks, we will treat our APIs as end-user features and use category=FutureWarning instead of the default DeprecationWarning. PendingDeprecationWarning is not used.
      • For pybind11, a deprecated function must be rewrapped in pure Python using function = deprecated(reason="why")(function).  If only one overload of a set is being deprecated, that should be stated in the reason string.  Over-warning is considered better than under-warning in this case.

      The deprecated package is available from PyPI with an MIT license. Its sphinx subpackage provides an automated way of updating docstrings in a Sphinx-compatible way to highlight the deprecated status. It has wrapt as a dependency, which is not yet in our conda environment.

      The deprecation package is also available from PyPI (with an Apache 2.0 license), auto-modifies docstrings, and has a single dependency (packaging). It has some nice features like ensuring that tests stop passing when code is supposed to be removed. But it is a little more verbose (requires current_version=_version_) and does not provide control over the warning class. In the future, we might try to incorporate features from deprecation into deprecated and push them upstream.

      For C++ code, I propose that the C++14 [[deprecated("why")]] syntax be used. This can be applied to a function, variable, or type. It should appear on a line of its own.

        Attachments

          Issue Links

            Activity

            Hide
            ktl Kian-Tat Lim added a comment -

            I have made some more changes to respond to the above comments.

            Show
            ktl Kian-Tat Lim added a comment - I have made some more changes to respond to the above comments.
            Hide
            tjenness Tim Jenness added a comment -

            I like the proposed plan. I assume we only really deprecate things that made it into formal releases? If something turns up in weekly N and we decide to change it in weekly N+4, do we need to keep it around with deprecation notices if we didn't make a release?

            Show
            tjenness Tim Jenness added a comment - I like the proposed plan. I assume we only really deprecate things that made it into formal releases? If something turns up in weekly N and we decide to change it in weekly N+4, do we need to keep it around with deprecation notices if we didn't make a release?
            Hide
            ktl Kian-Tat Lim added a comment -

            I think deciding what to deprecate using this procedure and what to simply change is a feature-specific determination that should be left for change RFCs and not an overall inflexible policy.

            Show
            ktl Kian-Tat Lim added a comment - I think deciding what to deprecate using this procedure and what to simply change is a feature-specific determination that should be left for change RFCs and not an overall inflexible policy.
            Hide
            tjenness Tim Jenness added a comment -

            Krzysztof Findeisen makeVisitInfo seems to be called in two places (pipe_drivers and obs_sdss) so we should probably fix those two calls and delete the function. The SourceTable method was deprecated 5 months ago so can't be deleted just yet. I would hope adoption of standard deprecation system would be applied to things that have already been deprecated without needing an RFC.

            Show
            tjenness Tim Jenness added a comment - Krzysztof Findeisen makeVisitInfo seems to be called in two places (pipe_drivers and obs_sdss) so we should probably fix those two calls and delete the function. The SourceTable method was deprecated 5 months ago so can't be deleted just yet. I would hope adoption of standard deprecation system would be applied to things that have already been deprecated without needing an RFC.
            Hide
            ktl Kian-Tat Lim added a comment -

            This seems to have converged to a workable consensus.

            Show
            ktl Kian-Tat Lim added a comment - This seems to have converged to a workable consensus.

              People

              • Assignee:
                ktl Kian-Tat Lim
                Reporter:
                ktl Kian-Tat Lim
                Watchers:
                Gabriele Comoretto, John Parejko, John Swinbank, Jonathan Sick, Kian-Tat Lim, Krzysztof Findeisen, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Planned End:

                  Summary Panel