MAINT make test_docstrings.py a regular test file#21899
MAINT make test_docstrings.py a regular test file#21899ogrisel merged 1 commit intoscikit-learn:mainfrom
Conversation
| obj = getattr(estimator, method) | ||
| try: | ||
| obj_signature = signature(obj) | ||
| obj_signature = str(signature(obj)) |
There was a problem hiding this comment.
Note to reviewers: this change was needed to make mypy happier.
thomasjpfan
left a comment
There was a problem hiding this comment.
Removing test_docstring.sh is nice. I am happy with this change. LGTM
|
The original motivation from @rth: "One point is that this needs numpydoc master, and generally I would rather see it as an optional linter than unit tests. Currently this validation is run in one of Azure CI jobs, maybe there is a better way to do it. The code style in the tests can also probably be improved." I don't have a strong opinion but since we tend to include our consistency checks in the test suite I don't mind adding this one as well. |
It's now using a released version of numpydoc. So at least that specific concern has been raised. For the other points, I think having too much complexity in our CI setup is detrimental, both for maintenance and for understanding by users reading the report. I checked in the Azure test report view that the tests are being executed as expected by the |
I am not sure what was the original motivation to keep the
test_docstrings.pyfile outside of thesklearnfolder (under themaint_tools/folder) but in retrospect I find it confusing.I think it should be treated as a regular test file.