[ENH] in suite tests, add test for docstring examples - #2374
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances the estimator “suite tests” for both v1 and v2 by adding checks intended to ensure forecaster classes and their corresponding package classes contain doctest-style examples in docstrings, and by running doctests on the package classes (in addition to the model classes).
Changes:
- Add
test_class_has_doctest_exampleto require doctest-style examples in model class docstrings. - Add
test_pkg_has_doctest_exampleto require doctest-style examples in package class docstrings. - Add
test_pkg_doctest_examplesto execute doctests for the package classes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
pytorch_forecasting/tests/test_all_estimators.py |
Adds doctest presence checks for v1 forecaster classes/packages and runs doctests for package classes. |
pytorch_forecasting/tests/test_all_v2/test_all_estimators_v2.py |
Adds doctest presence checks for v2 forecaster classes/packages and runs doctests for package classes. |
Suppressed comments (2)
pytorch_forecasting/tests/test_all_estimators.py:390
test_pkg_has_doctest_examplehas the same issue as the class variant: it requires>>>in every package class docstring, but current_pkgclasses have one-line docstrings (e.g.pytorch_forecasting/models/nhits/_nhits_pkg.py) and no>>>anywhere inpytorch_forecasting/models/**/_*pkg*.py. This will make the new test fail across the board.
Consider detecting actual doctest examples via doctest.DocTestFinder() and skipping when none exist (or, if the goal is to enforce examples, this PR also needs to add doctest examples to the package docstrings).
def test_pkg_has_doctest_example(self, object_pkg):
"""Check that the package has a docstring, with doctest example in it."""
docstring = object_pkg.__doc__
assert docstring is not None, f"{object_pkg.__name__} has no docstring"
msg = f"{object_pkg.__name__} docstring has no doctest example"
assert ">>>" in docstring, msg
pytorch_forecasting/tests/test_all_v2/test_all_estimators_v2.py:47
test_pkg_has_doctest_examplewill also fail for v2 package classes: there are no doctest prompts (>>>) in the current package class docstrings (includingBase_pkg). Either add doctest examples to those docstrings as part of this PR, or change this test to detect actual doctest examples and skip when none exist.
def test_pkg_has_doctest_example(self, object_pkg):
"""Check that the package has a docstring, with doctest example in it."""
docstring = object_pkg.__doc__
assert docstring is not None, f"{object_pkg.__name__} has no docstring"
msg = f"{object_pkg.__name__} docstring has no doctest example"
assert ">>>" in docstring, msg
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
We need to add the examples as well, I think we should branch off from this PR and do that first? and then merge both these PR in progression so that main doesnt fail! (Although adding examples to all the models will take some effort!) Raised an issue #2377 |
|
|
||
| run_doctest(object_class, name=f"class {object_class.__name__}") | ||
|
|
||
| def test_pkg_has_doctest_example(self, object_pkg): |
There was a problem hiding this comment.
should v1 model_pkg has usage examples? I think there it is just used as a class for testing no?? It is not used by the user
This PR adds, in the v1 and v2 suite tests:
test_class_has_doctest_exampleswhich checks that the model class has a docstring code example (doctest)test_pkg_has_doctest_examplewhich does the same for the respective package class, i.e., that it has a docstring code example (doctest)test_pkg_doctest_exampleswhich tests that this docstring code example, for the package class, runs without exceptions and has expected outputIn particular, this PR also closes a gap in test coverage, which left the doctests in the package classes untested.