From e7646dafff6126695a215b3cd14f8638dd4e6ab1 Mon Sep 17 00:00:00 2001 From: "Philipp A." Date: Fri, 11 Sep 2026 09:25:18 +0200 Subject: [PATCH] fix: correctly add deprecation for empty param --- .pre-commit-config.yaml | 1 + CHANGELOG.md | 8 ++ src/scverse_misc/sphinx_ext/__init__.py | 9 +- tests/deprecation_decorator/test_sphinx.py | 105 +++++++++++++++++---- 4 files changed, 98 insertions(+), 25 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 52f686d..67f01fe 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -48,6 +48,7 @@ repos: - pytest - sphinx - sphinxcontrib-katex + - types-docutils - types-PyYAML - repo: https://github.com/zizmorcore/zizmor-pre-commit rev: cef8b8350da46d8114c7e6b7272aebdccdf193ce # frozen: v1.30.0 diff --git a/CHANGELOG.md b/CHANGELOG.md index b4846a0..69d8b38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,13 @@ and this project adheres to [Semantic Versioning][]. [keep a changelog]: https://keepachangelog.com/ [semantic versioning]: https://semver.org/spec/ +## [0.1.6] + +### Fixed + +- `deprecated_arg` now indents the `version-deprecated` directive correctly for parameters without a description + (previously it was rendered as a separate parameter). + ## [0.1.5] ### Added @@ -128,6 +135,7 @@ and this project adheres to [Semantic Versioning][]. - Initial release +[0.1.6]: https://github.com/scverse/scverse-misc/releases/tag/v0.1.6 [0.1.5]: https://github.com/scverse/scverse-misc/releases/tag/v0.1.5 [0.1.4]: https://github.com/scverse/scverse-misc/releases/tag/v0.1.4 [0.1.3]: https://github.com/scverse/scverse-misc/releases/tag/v0.1.3 diff --git a/src/scverse_misc/sphinx_ext/__init__.py b/src/scverse_misc/sphinx_ext/__init__.py index 480def7..2c4297a 100644 --- a/src/scverse_misc/sphinx_ext/__init__.py +++ b/src/scverse_misc/sphinx_ext/__init__.py @@ -200,13 +200,12 @@ def _process_deprecated_args(app: Sphinx, deprecations: list[deprecated_arg], li if len(deprecation.msg): docmsg += f"\n{textwrap.indent(deprecation.msg, ' ')}" - docmsg_lines = docmsg.splitlines() - if len(docmsg_lines) > 1: - docmsg = f"\n{indentation}".join(docmsg_lines) - if desc is None: - edits.insert(par.range.end, f"\n{docmsg}") + # the insertion point is at the end of the name line, so indent every line + edits.insert(par.range.end, f"\n{textwrap.indent(docmsg, indentation)}") else: + # the insertion point is already indented, so only indent continuation lines + docmsg = f"\n{indentation}".join(docmsg.splitlines()) edits.replace(desc.range, f"{docmsg}\n\n{indentation}{desc.text}") lines[:] = edits.apply().splitlines() diff --git a/tests/deprecation_decorator/test_sphinx.py b/tests/deprecation_decorator/test_sphinx.py index a589cf3..a4f673f 100644 --- a/tests/deprecation_decorator/test_sphinx.py +++ b/tests/deprecation_decorator/test_sphinx.py @@ -1,6 +1,7 @@ from __future__ import annotations import inspect +import textwrap import warnings from collections.abc import Callable from typing import TYPE_CHECKING, Literal @@ -8,21 +9,59 @@ import pytest pytest.importorskip("scverse_misc.sphinx_ext") +from docutils import nodes +from sphinx import addnodes + from scverse_misc import Deprecation, deprecated_arg, sphinx_ext from scverse_misc.constants import ATTR_DEPRECATED, ATTR_DEPRECATED_ARG if TYPE_CHECKING: from sphinx.application import Sphinx from sphinx.ext.napoleon import GoogleDocstring, NumpyDocstring # type: ignore[attr-defined] - - -@pytest.fixture(scope="session", params=["no_docstring", "short", "long_googlestyle", "long_numpystyle"]) + from sphinx.testing.util import SphinxTestApp + + +@pytest.fixture( + scope="session", + params=[ + "no_docstring", + "short", + "long_googlestyle", + "long_numpystyle", + "no_param_desc_googlestyle", + "no_param_desc_numpystyle", + ], +) def docstring(request: pytest.FixtureRequest, docstring_style: Literal["google", "numpy"]) -> str | None: match request.param: case "no_docstring": return None case "short": return "Test function" + case "no_param_desc_numpystyle": + if docstring_style == "google": + pytest.skip("only google docstring parser enabled") + return """Test function + + Parameters + ---------- + positional_only_no_default + positional_only_default + positional_or_keyword_default + keyword_only_default + foobar + """ + case "no_param_desc_googlestyle": + if docstring_style == "numpy": + pytest.skip("only numpy docstring parser enabled") + return """Test function + + Args: + positional_only_no_default: + positional_only_default: + positional_or_keyword_default: + keyword_only_default: foobar + """ case "long_numpystyle": if docstring_style == "google": pytest.skip("only google docstring parser enabled") @@ -98,12 +137,42 @@ def test_deprecation_decorator( assert lines[offset + 1 : offset + 1 + len(msg_lines)] == msg_indented +def _words(node: nodes.Element) -> list[str]: + """Text of a node, ignoring whitespace and the separator before a parameter’s description.""" + return [word for word in node.astext().split() if word != "–"] + + +def _render_params( + app: SphinxTestApp, parser: type[GoogleDocstring | NumpyDocstring], **docstrings: list[str] +) -> dict[str, dict[str, nodes.list_item]]: + """Build each docstring as a `py:function` body, returning the rendered parameter items by name.""" + src = "Test\n====\n" + for name, lines in docstrings.items(): + body = textwrap.indent("\n".join(parser(lines).lines()), " ") + src += f"\n.. py:function:: {name}()\n\n{body}\n" + (app.srcdir / "index.rst").write_text(src) + app.build() + doctree = app.env.get_doctree("index") + + assert not list(doctree.findall(nodes.system_message)), "invalid rST" + return { + next(desc.findall(addnodes.desc_name)).astext(): { + next(item.findall(addnodes.literal_strong)).astext(): item for item in desc.findall(nodes.list_item) + } + for desc in doctree.findall(addnodes.desc) + } + + @pytest.mark.parametrize( "arg", ("positional_only_no_default", "positional_only_default", "positional_or_keyword_default", "keyword_only_default"), ) def test_deprecated_arg_decorator( - app: Sphinx, parser: type[GoogleDocstring | NumpyDocstring], func: Callable[..., int], msg: str | None, arg: str + app: SphinxTestApp, + parser: type[GoogleDocstring | NumpyDocstring], + func: Callable[..., int], + msg: str | None, + arg: str, ) -> None: deprecated_func = deprecated_arg(arg, Deprecation("2.718", msg or ""))(func) with pytest.warns(FutureWarning, match=f"{arg} is deprecated"): @@ -119,19 +188,15 @@ def test_deprecated_arg_decorator( lines = (inspect.getdoc(deprecated_func) or "").splitlines() sphinx_ext._process_deprecated_args(app, getattr(deprecated_func, ATTR_DEPRECATED_ARG), lines) - lines = parser(lines).lines() - - prefix = f":param {arg}:" - prefixlen = len(prefix) - lines = lines[next(i for i, line in enumerate(lines) if line.startswith(prefix)) :] - if msg is not None: - assert lines[1].strip() == ".. version-deprecated:: 2.718" - msg_lines = msg.splitlines() - for j, msg_line in enumerate(msg_lines): - assert lines[2 + j][prefixlen:] == f" {msg_line}" - assert not lines[2 + len(msg_lines)] - assert lines[3 + len(msg_lines)][:prefixlen] == " " * prefixlen - else: - assert lines[0] == f":param {arg}: .. version-deprecated:: 2.718" - assert not lines[1] - assert lines[2][:prefixlen] == " " * prefixlen + rendered = _render_params(app, parser, before=(inspect.getdoc(func) or "").splitlines(), after=lines) + + # an insufficiently indented directive would show up as an extra parameter + assert rendered["after"].keys() == rendered["before"].keys() + + notice = "Deprecated since version 2.718." if msg is None else f"Deprecated since version 2.718: {msg}" + for name, item in rendered["after"].items(): + notices = list(item.findall(addnodes.versionmodified)) + assert [n.astext().split() for n in notices] == ([notice.split()] if name == arg else []) + for n in notices: # the rest of the parameter must be untouched + n.parent.remove(n) + assert _words(item) == _words(rendered["before"][name])