TST Fixes docstring ordering and test_docstring_parameters #19048
Conversation
|
With the change to parametrize, it's hard to see the diff. Is the change to parametrize functional, i.e. is it helping to isolate errors? Otherwise, what's the big change here? |
| classes = [cls for cls in classes | ||
| if cls[1].__module__.startswith(name)] |
There was a problem hiding this comment.
@jnothman Here is the change. The parameterize was to make it easier for me to see all the errors at once.
I reverted the parameterize so the diff is smaller.
There was a problem hiding this comment.
Do I understand correctly that the problem was that name is e.g. sklearn.cross_decomposition while cls[1].__module__ is sklearn.cross_decomposition._pls?
There was a problem hiding this comment.
and if that's the case can we just do
classes = [cls for cls in classes if cls[1].__module__.startswith('sklearn')]?
There was a problem hiding this comment.
Yup that is nicer. Updated PR with your suggestion.
|
|
||
| Parameters | ||
| ---------- | ||
| df : pandas DataFrame |
There was a problem hiding this comment.
Do we want to enforce the style here: dataframe of shape ...?
|
@NicolasHug Issues like #19197 are not being caught because |
There was a problem hiding this comment.
Thanks @thomasjpfan , LGTM modulo confirmation on my question below
| @@ -84,7 +84,8 @@ def test_docstring_parameters(): | ||
| module = importlib.import_module(name) | ||
| classes = inspect.getmembers(module, inspect.isclass) | ||
| # Exclude imported classes |
There was a problem hiding this comment.
Nit:
| # Exclude imported classes | |
| # Exclude non-scikit-learn classes |
| classes = [cls for cls in classes | ||
| if cls[1].__module__.startswith(name)] |
There was a problem hiding this comment.
Do I understand correctly that the problem was that name is e.g. sklearn.cross_decomposition while cls[1].__module__ is sklearn.cross_decomposition._pls?
This PR fixes
test_docstring_parametersso that it actually runs and the corresponding failures it sees in the docstrings.