mjhajharia / deprecat Goto Github PK

TODO for self

Minimal version of the idea is to have a kwarg in the sphinx decorator that lets you choose the kind of docstrings you have with args being numpydoc, google. Currently, only Numpydoc is supported as it is more common in a lot of open source software, but sphinx supports google docstrings as well.

You'd only want to change the regex for finding the position in the docstring.

There is a https://deprecat.readthedocs.io/en/latest/source/usage.html#docstring section which is hardcoded in rST instead of showing the actual output.

I would try to create a showcase module inside the library that is not used by it nor exposed when imported but has one example of each deprecation type, class, function, method, kwarg... and then create a showcase api docs. I think this will serve also as a rudimentary test that the output looks as desired when doing changes in a way that automated tests can't

All the tests are unmodified and taken as it is from github:tantale/deprecated, we need to change them to suit the new features such as deprecated_args

On a method that doesn't have a docstring at all, or is missing the expected section, the search result is None:

search = re.search("Parameters[\s]*\n[\s]*----------", docstring)
params_string = docstring[search.start():search.end()]

Traceback

...\deprecat\classic.py:217: in deprecat
    return wrapper_function(wrapped)
...\wrapt\decorators.py:417: in _wrapper
    return _build(target_wrapped, target_wrapper, _enabled, adapter)
...\wrapt\decorators.py:207: in _build
    adapter = adapter(wrapped)
...\deprecat\sphinx.py:145: in __call__
    params_string = docstring[search.start():search.end()]
E   AttributeError: 'NoneType' object has no attribute 'start'

In addition to version= giving the "deprecated in" version number, it would be nice to be able to specify a "will be removed in" version number.

You could add another argument, e.g. say with remove_version or deadline_version.

I can do a PR if nobody who knows the code better has the time.

2.1.2 has been tagged here on github: https://github.com/mjhajharia/deprecat/releases/tag/v2.1.2

But hasn't been pushed to PyPI:

> pip install --upgrade --dry-run deprecat
Collecting deprecat
  ...
Would install deprecat-2.1.1 wrapt-1.16.0

As a way to keep the verbosity to minimum (in log and in the code), I think this would be nice to be able to left the "reason" empty (e.g., if the reason is too obvious or too long).

@deprecat(deprecated_args={'a':{'version':'4.0'}, 'b':{'version':'3.0'}})
def multiply(a, b, c):
    return 42

Currently the code above raises an error:

Traceback (most recent call last):
File "", line 1, in
File "/opt/homebrew/lib/python3.11/site-packages/deprecat/classic.py", line 205, in wrapper_function
msg = adapter.get_deprecated_msg(wrapped_, instance_, kwargs_)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/opt/homebrew/lib/python3.11/site-packages/deprecat/classic.py", line 120, in get_deprecated_msg
if self.deprecated_args[arg]['reason']:
~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^

While, IMO, it would be great to show

>>> multiply(a=1,b=2,c=3)
:1: DeprecationWarning: Call to deprecated Parameter a. -- Deprecated since v4.0.
:1: DeprecationWarning: Call to deprecated Parameter b. -- Deprecated since v3.0.
42

If needed, I'd be happy to help and implement this feature.

Very similar to #5 this line assumes that the deprecated kwarg is in the docstring:

description_start = re.search(f"\n{indent}{arg}\s*:", params_section).end()

...but again the search returns None if it's not found.

Currently the deprecation warning is only shown for kwargs. I think that doing the same for args would be interesting (either by config or by default).

@deprecat(deprecated_args={'a':{'version':'4.0', 'reason': ''}, 'b':{'version':'3.0',  'reason': ''}})
def multiply(a, b, c):
    return 42
    
multiply(1,2,3)
multiply(a=1,b=2,c=3)

Wouldn't it be more consistent to display the deprecation at the first call? The deprecated args have actually been called. Currently the output is

>>> multiply(1,2,3)
42
>>> multiply(a=1,b=2,c=3)
<stdin>:1: DeprecationWarning: Call to deprecated Parameter a. -- Deprecated since v4.0.
<stdin>:1: DeprecationWarning: Call to deprecated Parameter b. -- Deprecated since v3.0.
42

will explain later, dropping these as tasks for me