Repository navigation
[ci] [python-package] enforce 'numpydoc-validation' and 'pydoclint' checks - #7478
Conversation
| Returns | ||
| ------- | ||
| infos : tuple | ||
| tuple |
There was a problem hiding this comment.
numpydoc-validation would really prefer that functions that return a single object not name the returned object. See https://numpydoc.readthedocs.io/en/latest/format.html#returns
| list | ||
| Names of all parameters. | ||
| """ | ||
| """ # noqa: DOC105 |
There was a problem hiding this comment.
pydoclint directly compares the text of the type hint in the docstring to the text of the type hint in the code. This produces lots of false positives for lightgbm, for example where Python code has Path and the docstring has pathlib.Path.
I've requested a way to globally ignore checks, similar to ruff's "ignore" (jsh9/pydoclint#228 (comment)). But until then, I think it's not too bad to have these noqa comments everywhere. We don't add new public interfaces with new docstrings that often.
| eval_init_score: Optional[List[_DaskCollection]] = None, | ||
| eval_metric: Optional[_LGBM_ScikitEvalMetricType] = None, | ||
| *, | ||
| categorical_feature: _LGBM_CategoricalFeatureConfiguration = "auto", |
There was a problem hiding this comment.
These had been in the docstrings inherited from the sklearn estimators but not actually written out in the code.
They were previously passed through **kwargs, and I'm just proposing promoting them to the signature:
- to help with tab completion and discoverability via
help() - to satisfy linter warnings about the "Parameters" section not matching the signature
borchero
left a comment
There was a problem hiding this comment.
Have you checked out pydocstyle? It's neat because you can integrate it directly with ruff; wouldn't need additional pre-commit hooks.
|
This project already uses the LightGBM/python-package/pyproject.toml Lines 164 to 165 in 439136e That didn't catch any of the issues caught by These are complementary tools.
There's some overlap between I think we should have all 3 of these things enabled here. It would have avoided bugs like missing some docs updates when |
borchero
left a comment
There was a problem hiding this comment.
All right, thanks for the explanation @jameslamb!
|
Thanks very much for considering it and carefully reviewing! |
Contributes to #7470
Proposes adding
numpydoc-validationandpydoclint, 2 linters that enforce standardization and correctness of Python docstrings. These caught a few issues (likeeval_{X,y}missing from the Dask docstrings.Both are fast enough to not be noticeable.
I think they'll help a lot with consistency and standardization, and relieve a small bit of review burden.
Notes for Reviewers
I tried to choose configurations for these tools that as closely as possible match the project's current conventions. See inline comments below.
I've added this branch (temporarily) to readthedocs configuration.