-
Notifications
You must be signed in to change notification settings - Fork 0
add decorator for deprecating function arguments (closes #18) #25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,7 @@ Types used by the former: | |
| :toctree: generated | ||
|
|
||
| deprecated | ||
| deprecated_arg | ||
| Deprecation | ||
| ``` | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,11 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import inspect | ||
| import sys | ||
| from inspect import getdoc | ||
| from functools import wraps | ||
| from textwrap import indent | ||
| from typing import TYPE_CHECKING, LiteralString | ||
| from warnings import warn | ||
|
|
||
| if sys.version_info >= (3, 13): | ||
| from warnings import deprecated as _deprecated | ||
|
|
@@ -13,7 +16,7 @@ | |
| from collections.abc import Callable | ||
|
|
||
|
|
||
| __all__ = ["deprecated", "Deprecation"] | ||
| __all__ = ["deprecated", "deprecated_arg", "Deprecation"] | ||
|
|
||
|
|
||
| class Deprecation(str): | ||
|
|
@@ -56,7 +59,7 @@ def decorate(func: F) -> F: | |
| kind = "function" if func.__name__ == func.__qualname__ else "method" | ||
| warnmsg = f"The {kind} {func.__name__} is deprecated and will be removed in the future." | ||
|
|
||
| doc = getdoc(func) | ||
| doc = inspect.getdoc(func) | ||
| docmsg = f".. version-deprecated:: {msg.version_deprecated}" | ||
| if len(msg): | ||
| docmsg += f"\n {msg}" | ||
|
|
@@ -79,3 +82,91 @@ def decorate(func: F) -> F: | |
| deprecated = _deprecated | ||
| else: | ||
| deprecated = _deprecated_at | ||
|
|
||
|
|
||
| def deprecated_arg[**P, R]( | ||
| arg: LiteralString, msg: Deprecation, *, category: type[Warning] = FutureWarning, stacklevel: int = 1 | ||
| ) -> Callable[[Callable[P, R]], Callable[P, R]]: | ||
| """Decorator to indicate that a function argument is deprecated. | ||
|
|
||
| Emits a warning when the decorated function is called with the deprecated argument and addtionally modifies the | ||
| docstring to include a deprecation notice. | ||
|
|
||
| Args: | ||
| arg: The deprecated argument. | ||
| msg: The deprecation message. | ||
| category: The category of the warning that will be emitted at runtime. | ||
| stacklevel: The stack level of the warning. | ||
|
|
||
| Examples: | ||
| >>> @deprecated_arg("bar", Deprecation("0.2", "The functionality has moved to the baz() function.")) | ||
| ... def foo(baz, bar=1): | ||
| ... pass | ||
| """ | ||
|
|
||
| def decorate(func: Callable[P, R]) -> Callable[P, R]: | ||
| warnmsg = f"The argument {arg} is deprecated and will be removed in the future." | ||
| doc = inspect.getdoc(func) | ||
| docmsg = f" .. version-deprecated:: {msg.version_deprecated}" | ||
|
|
||
| if len(msg): | ||
| docmsg += f"\n {msg}" | ||
| warnmsg += f" {msg}" | ||
|
|
||
| if doc is not None: | ||
| lines = doc.splitlines() | ||
| docstring_style = None | ||
| in_arg_section = False | ||
| in_arg_header = False | ||
| for i, line in enumerate(lines): | ||
| if in_arg_header: | ||
| in_arg_header = False | ||
| continue | ||
| elif not in_arg_section and line == "Parameters" and lines[i + 1] == "----------": | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| docstring_style = "numpy" | ||
| in_arg_section = True | ||
| in_arg_header = True | ||
| elif not in_arg_section and line == "Args:": | ||
| docstring_style = "google" | ||
| in_arg_section = True | ||
| docmsg = indent(docmsg, " ") | ||
| elif in_arg_section: | ||
| if docstring_style == "numpy" and line == arg: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in |
||
| doc = "\n".join(lines[: i + 1]) + f"\n{docmsg}\n\n" + "\n".join(lines[i + 1 :]) | ||
| break | ||
| elif docstring_style == "google" and line.startswith(prefix := f" {arg}: "): | ||
| doc = ( | ||
| "\n".join(lines[:i]) | ||
| + f"\n{prefix}\n{docmsg}\n\n {line[len(prefix) :]}\n" | ||
| + "\n".join(lines[i + 1 :]) | ||
| ) | ||
| break | ||
| elif ( | ||
| docstring_style == "numpy" | ||
| and set(line.strip()) == {"-"} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a comment about what that singular |
||
| or docstring_style == "google" | ||
| and not line[0].isspace() | ||
| ): # next section, arg not documented | ||
| break | ||
| func.__doc__ = doc | ||
|
|
||
| sig = inspect.signature(func) | ||
| param = sig.parameters[arg] | ||
|
|
||
| @wraps(func) | ||
| def wrapped(*args: P.args, **kwargs: P.kwargs) -> R: | ||
| if ( | ||
| param.kind in (inspect.Parameter.KEYWORD_ONLY, inspect.Parameter.POSITIONAL_OR_KEYWORD) | ||
| and arg in kwargs | ||
| ): | ||
| warn(warnmsg, category=category, stacklevel=stacklevel) | ||
| else: | ||
| bound = sig.bind(*args, **kwargs) | ||
| if arg in bound.arguments and bound.arguments[arg] != param.default: | ||
| warn(warnmsg, category=category, stacklevel=stacklevel) | ||
|
Comment on lines
+162
to
+166
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The default
I’m 90% sure you need to do |
||
|
|
||
| return func(*args, **kwargs) | ||
|
|
||
| return wrapped | ||
|
|
||
| return decorate | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,52 +1,85 @@ | ||
| import inspect | ||
| import warnings | ||
| from collections.abc import Callable | ||
| from typing import cast | ||
| from typing import Literal, cast, get_args | ||
|
|
||
| import pytest | ||
| from sphinx.ext.napoleon import GoogleDocstring, NumpyDocstring # type: ignore[attr-defined] | ||
|
|
||
| from scverse_misc import Deprecation, deprecated | ||
| from scverse_misc import Deprecation, deprecated, deprecated_arg | ||
|
|
||
|
|
||
| @pytest.fixture(params=[pytest.param(None, id="no_message"), pytest.param("Test message.", id="message")]) | ||
| def msg(request: pytest.FixtureRequest) -> str | None: | ||
| return cast(str | None, request.param) | ||
|
|
||
|
|
||
| @pytest.fixture( | ||
| params=[ | ||
| pytest.param(None, id="no_docstring"), | ||
| pytest.param("Test function", id="short"), | ||
| pytest.param( | ||
| """Test function | ||
| type DocstringStyles = Literal["no_docstring", "short", "long_numpystyle", "long_googlestyle"] | ||
|
|
||
|
|
||
| @pytest.fixture(params=get_args(DocstringStyles.__value__)) | ||
| def docstring_style(request: pytest.FixtureRequest) -> DocstringStyles: | ||
| return cast(DocstringStyles, request.param) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def docstring(docstring_style: DocstringStyles) -> str | None: | ||
| match docstring_style: | ||
| case "no_docstring": | ||
| return None | ||
| case "short": | ||
| return "Test function" | ||
| case "long_numpystyle": | ||
| return """Test function | ||
|
|
||
| This is a test. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| foo | ||
| positional_only_no_default | ||
| foo | ||
| positional_only_default | ||
| bar | ||
| bar | ||
| positional_or_keyword_default | ||
| baz | ||
| """, | ||
| id="long", | ||
| ), | ||
| ] | ||
| ) | ||
| def docstring(request: pytest.FixtureRequest) -> str | None: | ||
| return cast(str | None, request.param) | ||
| keyword_only_default | ||
| foobar | ||
| """ | ||
| case "long_googlestyle": | ||
| return """Test function | ||
|
|
||
| This is a test. | ||
|
|
||
| Args: | ||
| positional_only_no_default: foo | ||
| positional_only_default: bar | ||
| positional_or_keyword_default: baz | ||
| keyword_only_default: foobar | ||
| """ | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def deprecated_func(msg: str | None, docstring: str | None) -> Callable[[int, int], int]: | ||
| def func(foo: int, bar: int) -> int: | ||
| def func(msg: str | None, docstring: str | None) -> Callable[..., int]: | ||
| def _func( | ||
| positional_only_no_default: int, | ||
| positional_only_default: int = 1337, | ||
| /, | ||
| positional_or_keyword_default: int = 42, | ||
| *, | ||
| keyword_only_default: float = 3.1415, | ||
| ) -> int: | ||
| return 42 | ||
|
|
||
| func.__doc__ = docstring | ||
| _func.__doc__ = docstring | ||
| return _func | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def deprecated_func(msg: str | None, func: Callable[..., int]) -> Callable[..., int]: | ||
| return deprecated(Deprecation("foo", msg or ""))(func) | ||
|
|
||
|
|
||
| def test_deprecation_decorator( | ||
| deprecated_func: Callable[[int, int], int], docstring: str | None, msg: str | None | ||
| ) -> None: | ||
| def test_deprecation_decorator(deprecated_func: Callable[..., int], docstring: str | None, msg: str | None) -> None: | ||
| with pytest.warns(FutureWarning, match="deprecated"): | ||
| assert deprecated_func(1, 2) == 42 | ||
|
|
||
|
|
@@ -63,3 +96,45 @@ def test_deprecation_decorator( | |
| assert len(lines) == 3 or not lines[3].startswith(" ") | ||
| else: | ||
| assert lines[3] == f" {msg}" | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "arg", | ||
| ("positional_only_no_default", "positional_only_default", "positional_or_keyword_default", "keyword_only_default"), | ||
| ) | ||
| def test_deprecated_arg_decorator( | ||
| func: Callable[..., int], msg: str | None, arg: str, docstring_style: DocstringStyles | ||
| ) -> None: | ||
| deprecated_func = deprecated_arg(arg, Deprecation("2.718", msg or ""))(func) | ||
| with pytest.warns(FutureWarning, match=f"{arg} is deprecated"): | ||
| assert deprecated_func(1, 2, 3, keyword_only_default=4.0) == 42 | ||
|
|
||
| if arg != "positional_only_no_default": | ||
| with warnings.catch_warnings(): | ||
| warnings.simplefilter("error") | ||
| assert deprecated_func(1) == 42 | ||
|
|
||
| parser: type[NumpyDocstring] | type[GoogleDocstring] | None = None | ||
| if docstring_style == "long_numpystyle": | ||
| parser = NumpyDocstring | ||
| elif docstring_style == "long_googlestyle": | ||
| parser = GoogleDocstring | ||
|
|
||
| if parser is None: | ||
| return | ||
|
|
||
| lines = parser(inspect.getdoc(deprecated_func) or "").lines() | ||
|
|
||
| for i, line in enumerate(lines): | ||
| if line.startswith(prefix := f":param {arg}: "): | ||
| prefixlen = len(prefix) | ||
| if msg is not None: | ||
| stripped = lines[i + 1].strip() | ||
| assert stripped == ".. version-deprecated:: 2.718" | ||
| assert lines[i + 2][prefixlen:] == f" {msg}" | ||
| assert not lines[i + 3] | ||
| assert lines[i + 4][:prefixlen] == " " * prefixlen | ||
| else: | ||
| assert line == f":param {arg}: .. version-deprecated:: 2.718" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does that render as expected? |
||
| assert not lines[i + 1] | ||
| assert lines[i + 2][:prefixlen] == " " * prefixlen | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation handling looks fragile since you seem to expect exactly 4 spaces here
sphinx.ext.napoleonmight be less strict.So really, any amount of indentation should be handled here without breaking. I don’t even know how that would work if someone uses an inline google-style parameter description:
would then have to become this?
Args: foo: the description .. version-deprecated:: ...or is mixed inline and indented OK?