Use numpydoc on public API of Parcels #2474
Conversation
VeckoTheGecko
force-pushed
the
numpydoc-ast
branch
from
9e0dfa3 to
419b0ef
Compare
There was a problem hiding this comment.
this does create another folder tools in the root of the repository. I think its fine here - and also allows us to add any other tools we need in future. Putting it in .github didn't really seem appropriate since it doesn't have to do with github or really with CI
Though it does bump the README down one item. Open to ideas
There was a problem hiding this comment.
Yes, not ideal to add another folder. But we could/should get rid of tests-v3 soon(ish) so that reduced the folder structure by one line ;-)
Any other files/folders we can move?
There was a problem hiding this comment.
Any other files/folders we can move?
Just had a look - I don't think so
There was a problem hiding this comment.
I went ahead and categorised these - mapping out future work that can be done on the docstrings. Happy to go over these together if you want @erikvansebille
| "UP", # pyupgrade | ||
| "LOG", # logging | ||
| "ICN", # import conventions | ||
| "G", # logging-format |
erikvansebille
left a comment
erikvansebille
left a comment
There was a problem hiding this comment.
Looks good to me! Fixing the warnings will be another PR, I assume?
Yes, definitely. I also need to check that the docs are flagging warnings correctly (in a way that we can address them) - also another PR I'll probably also make another issue for the docstrings - I can imagine getting them up to scratch will be over a longer timespan |
They have a new release
- Create tools/numpydoc-public-api.py script - Update pixi.toml to expose this script via a task numpydoc-lint - Update conf.py to work from the shared tool data
We should properly consider this later when we add logging to other parts of the code - but from the outset it doesn't seem like that useful a rule.
for more information, see https://pre-commit.ci
VeckoTheGecko
force-pushed
the
numpydoc-ast
branch
from
58b39ab to
0fb7ec1
Compare
2 participants
We should use
numpydocto validate docstrings that are part of our public API (validating all the API would be very overkill). ThenumpydocCLI tool, as well asruff, do not have support for delineating what is part of the public and private API since they operate on the file level. You can specify which files to run on - but this doesn't work for us (or many other packages) which import from private modules to expose items in the public API. These tools cannot - for example - import a package and traverse down the package's__all__variables.Numpydoc does however have a Sphinx integration which does work quite well for linting the public API (after all, this public API is showed in the API reference, and the goal of numpydoc is to provide high quality user documentation). Running a full Sphinx build (which in our case also runs our notebooks) is a lot just to find a missing comma in a docstring.
This PR does the following:
tools/numpydoc-public-api.pyscript (use-v,-vvetc. to increase the verbosity of the script)parcelswalks down the package structure following the__all__variables to discover the public APIpixi.tomlto expose this script via a tasknumpydoc-lintconf.pyto work from the shared tool dataWe need to review the errors to skip in the
numpydoc_skip_errorssection oftools/tool-data.tomlto see which rules we want to ignore. Currently we ignore them all.Unfortunately I don't think we can make this into a pre-commit hook, as the script requires installing Parcels.
I think this should will be sufficient. Closes #1971
This PR is also the result of some of the discussions I had with scipy folks - as alluded to in #1971 (comment)