doclint: reject non-numpydoc docstrings on changed files - #5345
Conversation
numpydoc lint is AST-based and already a docs dependency; Sphinx field lists and Google-style headings are both invisible to its parser, so PR01/PR02/PR03 flag them without a bespoke regex. doclint runs it over only the *.py files a branch changed against $BASE (default origin/main), and is wired into `make lint`. flake8 and pylint move into the existing [docs] extra so a fresh checkout's `make lint` has something to run; CI's lint job still installs them directly, since it runs on a bare runner with no PETSc build. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
f66bd82 to
6350533
Compare
|
How does this work if we need to make a deliberate decision to violate numpydoc in some instance? |
I think you can violate numpydoc in this approach. This PR only forbids non-numpydoc in new lines by looking at the diff against main or release (whichever is the closest). A deliberate violation can be accepted if we "merge with prejudice" even if CI reports a doclint failure. We will not continue to error on historical violations. As far as I understand, currently there is no step in However, if we want mechanism, not policy, we can have loud warnings instead of failures, with the hope that a human or LLM contributor cares enough to address them. But if our code contribution guide says this is our policy, it'd be good for it to be enforced automatically, to allow the review to be focused on the Mathematics and the actual purpose of the code. |
|
What do we want to keep from here? |
Adds
make doclint: numpydoc-lints the*.pyfiles changed by a branch against$BASE, catching Sphinx/Google-style docstrings via PR01-PR03. Wired intomake lintand CI's lint job (now needsfetch-depth: 0for the diff).flake8/pylintmove into thedocsextra so a fresh checkout has something to lint with.CI passes
BASEexplicitly (main vs release, from the PR's base/label). Locally,BASEdefaults by picking whichever oforigin/main/origin/releaseforked fromHEADmore recently.No code touched, so based on
release.