Skip to content

ci: check docstring parameters against signatures (pydoclint, SIT-422) - #1306

Draft
jan-glx wants to merge 1 commit into
scverse:mainfrom
jan-glx:jangleixner/sit-422-docstring-signature-check
Draft

jan-glx wants to merge 1 commit into
scverse:mainfrom
jan-glx:jangleixner/sit-422-docstring-signature-check

Conversation

@jan-glx

@jan-glx jan-glx commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #1303 (SIT-422).

Depends on #1311, which fixes the docstrings this check flags (split out so they can go in independently). This PR contains only the hook, its config and the # noqa comments below, so its pydoclint check fails until #1311 is merged.

Adds pydoclint as a pre-commit hook on src/, so CI catches docstrings that document parameters a function doesn't have, or miss ones it does (the #1273 / #1254 kind of drift).

Config ([tool.pydoclint] in pyproject.toml), kept narrow on purpose:

  • numpy style; class docstrings may document __init__ parameters (allow-init-docstring)
  • checks: missing or stale parameter names, and that Returns/Yields sections exist
  • not checked: types (they live in the signatures), argument order, raises
  • kwargs may be documented without stars, as the codebase already does
  • docstrings with only a summary (no Parameters section) are skipped, so terse private helpers aren't affected

Why pydoclint over numpydoc's PR01/PR02 validation: numpydoc also flags every docstring that has no Parameters section (~100 hits here, mostly private helpers). pydoclint only checks docstrings that do document parameters, so its findings are actual drift.

Intentional mismatches: ShapesModel.parse / PointsModel.parse document the parameters of their singledispatch overloads, and bounding_box_query's **kwargs exists only for singledispatch (no overload accepts extra arguments), so it is not documented (#1311 removes its stale entry). These get a # noqa: DOC10x; lint.external = ["DOC1"] stops ruff's RUF100 from flagging those noqa comments as unused.

As suggested in review, the hook in .pre-commit-config.yaml notes that its options live in [tool.pydoclint] in pyproject.toml.

@codecov

codecov Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.43%. Comparing base (bf43e7c) to head (9771c2b).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1306   +/-   ##
=======================================
  Coverage   91.43%   91.43%           
=======================================
  Files          53       53           
  Lines        8395     8395           
=======================================
  Hits         7676     7676           
  Misses        719      719           
Files with missing lines Coverage Δ
src/spatialdata/_core/query/spatial_query.py 95.01% <ø> (ø)
src/spatialdata/models/models.py 87.83% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jan-glx
jan-glx marked this pull request as ready for review November 9, 2026 20:30

@ajkswamy ajkswamy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this PR introduces a significant change to the precommit config, it would be good to get at least in principle approval from all other active developers before merging this

Comment thread .pre-commit-config.yaml Outdated
rev: 0.12.0
hooks:
- id: pydoclint
files: ^src/

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please document here that args of this hook live in pyproject.toml, which will help readability and maintenability.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment on the hook pointing to [tool.pydoclint] in pyproject.toml.

@jan-glx
jan-glx force-pushed the jangleixner/sit-422-docstring-signature-check branch from f29ebdc to 9dc47ef Compare November 11, 2026 09:59
@jan-glx
jan-glx marked this pull request as draft November 11, 2026 10:20
…-422)

Adds pydoclint as a pre-commit hook on src/, configured in [tool.pydoclint]
in pyproject.toml (noted on the hook, as asked in review). Builds on the
docstring fixes in jangleixner/sit-422-docstring-fixes.
@jan-glx
jan-glx force-pushed the jangleixner/sit-422-docstring-signature-check branch from 9dc47ef to 9771c2b Compare November 11, 2026 10:45
ajkswamy pushed a commit that referenced this pull request Nov 11, 2026
…422) (#1311)

Docstring-only fixes split out of #1306 (SIT-422), so they can go in
independently of the decision on the pydoclint hook. #1306 now contains
only the hook and its config, and needs this PR merged first for its
check to pass.

Found by running pydoclint over `src/`:
- `get_extent`: most parameters were filed under *Returns*, and the
return value was described wrongly
- `_bounding_box_mask_points`: a leftover `PLACEHOLDER` block, and the
shared min/max coordinate docs pasted in twice
- shared `min_coordinate`/`max_coordinate` docs: the first line rendered
4 spaces deeper than the rest (also on `main` in `BoundingBoxRequest`
and `bounding_box_query`), and `max_coordinate` now mentions the
`(n_axes,)` single-box shape like `min_coordinate` does
- *Returns*/*Notes* sections that didn't parse (indented body, `-------`
underline)
- parameter entries written as `name: type`, which numpydoc parses as a
parameter called `name: type`; the types already live in the signatures,
so only the names are kept
- undocumented parameters: `c_coords` (`blobs`), `suppress_z_warning`,
`sdata`, `zarr_path`, `intermediate_coordinate_systems`, …
- stale parameter docs (e.g. `bounding_box_query`'s dispatch-only
`**kwargs`), clarified `iterate_pyramid_levels` yields
- added missing Returns/Yields sections

The `# noqa` comments for the intentional mismatches (singledispatch
generics) are part of #1306, since they only matter with the hook.

No code changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci: check docstring parameters against signatures

2 participants