add decorator for deprecating function arguments (closes #18) by ilia-kats · Pull Request #25 · scverse/scverse-misc

ilia-kats · 2026-05-06T13:49:00Z

A warning is emitted if one of the following is true:

The deprecated argument can be passed by keyword and it is passed by keyword.
The deprecated argument can be passed by position, it is passed by position, and its value differs from the default.

codecov · 2026-05-06T13:50:42Z

Codecov Report

❌ Patch coverage is 98.18182% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 97.56%. Comparing base (e37825c) to head (1ac1a7f).

Files with missing lines	Patch %	Lines
src/scverse_misc/_deprecated.py	98.11%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #25      +/-   ##
==========================================
+ Coverage   97.47%   97.56%   +0.09%     
==========================================
  Files           5        5              
  Lines         238      288      +50     
==========================================
+ Hits          232      281      +49     
- Misses          6        7       +1

Files with missing lines	Coverage Δ
src/scverse_misc/__init__.py	`100.00% <100.00%> (ø)`
src/scverse_misc/_deprecated.py	`98.80% <98.11%> (-1.20%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

flying-sheep

Good functionality, but I’m concerned about the docstring modifications being too fragile

flying-sheep · 2026-05-07T08:38:55Z

+                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)


The default stacklevel=1 should function like in warnings.deprecated

If it is 1 (the default), the warning is emitted at the direct caller of the deprecated object;

I’m 90% sure you need to do warn(..., stacklevel=stacklevel+1) to achieve that here, but please verify.

flying-sheep · 2026-05-07T08:41:23Z

+    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}"


The indentation handling looks fragile since you seem to expect exactly 4 spaces here

Google docstrings are supposed to be indented by 2 or 4 spaces per level, but I assume that sphinx.ext.napoleon might be less strict.

reStructuredText and therefore numpy docstring indentation isn’t constrained even in style guides

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:

Args: foo: the description

would then have to become this?

Args: foo: the description .. version-deprecated:: ...

or is mixed inline and indented OK?

flying-sheep · 2026-05-07T08:46:55Z

+                if in_arg_header:
+                    in_arg_header = False
+                    continue
+                elif not in_arg_section and line == "Parameters" and lines[i + 1] == "----------":


There’s also https://numpydoc.readthedocs.io/en/latest/format.html#other-parameters

flying-sheep · 2026-05-07T09:35:57Z

+                assert not lines[i + 3]
+                assert lines[i + 4][:prefixlen] == " " * prefixlen
+            else:
+                assert line == f":param {arg}: .. version-deprecated:: 2.718"


does that render as expected?

flying-sheep · 2026-05-07T09:42:38Z

+                    in_arg_section = True
+                    docmsg = indent(docmsg, "    ")
+                elif in_arg_section:
+                    if docstring_style == "numpy" and line == arg:


in numpy style, there can be a : qualifier after the param name.

flying-sheep · 2026-05-07T09:43:33Z

+                        break
+                    elif (
+                        docstring_style == "numpy"
+                        and set(line.strip()) == {"-"}


Could you add a comment about what that singular - is about? I remember handling that too elsewhere, but I forget.

ilia-kats · 2026-05-07T11:08:30Z

@flying-sheep I think perhaps it would make sense at this point to add something like docstring-parser as a dependency. This will alow us to parse a docstring into a DocString object, edit the DocstringParam object, and render an updated docstring.

This would also simplify docstring handling throughoug scverse-misc.

EDIT: or maybe pydocstring-rs

flying-sheep · 2026-05-07T11:23:14Z

Before we do that, we should revisit the idea of not doing any docstring manipulation at runtime and doing that in a Sphinx plugin instead.

Don’t get me wrong, something like pydocstring-rs is probably lightweight, fast, and good enough to just depend on it, but I do think splitting responsibilities here might be good.

ilia-kats · 2026-05-07T11:28:35Z

How would the Sphinx-plugin work? The decorator adds e.g. __scverse_misc_deprecated_arg__ to the function and the plugin checks for it and modifies the docstring? That would then also require all packages that depend on scverse-misc to manually add the plugin to their Sphinx config?

flying-sheep · 2026-05-07T13:51:15Z

yup, but

we could basically add whatever dependencies and fanciness we want to scverse-misc[sphinx]
the runtime part would be agnostic in respect to docstring syntax – individual scverse packages could switch to non-rst docstrings and non-sphinx if they want (and could contribute a scverse-misc[zensical] plugin – or materialx or whatever comes out of the mkdocs-material mess in the end)
the separation between docs code and runtime code would benefit maintainability (provided we test the docs part nicely)

only question is if that would still integrate so neatly with autosummary-generate … worth an experiment

To come back to this PR – maybe we add the sphinx extra right now, add pydocstring-rs (or whatever you choose) behind it, and skip docstring modification if that can’t be imported?

A PR that adds usage of scverse_misc.deprecated_arg would then have to add scverse-misc[sphinx] to its docs dependency group to make it work, which doesn’t sound like aa huge burden.

grst · 2026-05-07T13:54:07Z

the separation between docs code and runtime code would benefit maintainability (provided we test the docs part nicely)

I think a disadvantage with having it in sphinx might also be that the docs shown in IDEs will not be respecting that?

flying-sheep · 2026-05-07T13:55:16Z

they don’t show it anyway, they only show static docstrings, not runtime docstrings.

ilia-kats requested review from flying-sheep and grst May 6, 2026 13:49

add decorator for deprecating function arguments (closes #18)

1ac1a7f

ilia-kats force-pushed the deprecated_arg branch from 6c0c524 to 1ac1a7f Compare May 6, 2026 15:28

ilia-kats linked an issue May 7, 2026 that may be closed by this pull request

deprecate only a specific argument of a function #18

Open

flying-sheep requested changes May 7, 2026

View reviewed changes

Conversation

ilia-kats commented May 6, 2026

Uh oh!

codecov Bot commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

flying-sheep left a comment

Choose a reason for hiding this comment

Uh oh!

flying-sheep May 7, 2026

Choose a reason for hiding this comment

Uh oh!

flying-sheep May 7, 2026

Choose a reason for hiding this comment

Uh oh!

flying-sheep May 7, 2026

Choose a reason for hiding this comment

Uh oh!

flying-sheep May 7, 2026

Choose a reason for hiding this comment

Uh oh!

flying-sheep May 7, 2026

Choose a reason for hiding this comment

Uh oh!

flying-sheep May 7, 2026

Choose a reason for hiding this comment

Uh oh!

ilia-kats commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flying-sheep commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ilia-kats commented May 7, 2026

Uh oh!

flying-sheep commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

grst commented May 7, 2026

Uh oh!

flying-sheep commented May 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented May 6, 2026 •

edited

Loading

ilia-kats commented May 7, 2026 •

edited

Loading

flying-sheep commented May 7, 2026 •

edited

Loading

flying-sheep commented May 7, 2026 •

edited

Loading