Skip to content

Function parametrize_with_checks does not accept classes #16707

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
TomDLT opened this issue Mar 16, 2020 · 2 comments · Fixed by #16709
Closed

Function parametrize_with_checks does not accept classes #16707

TomDLT opened this issue Mar 16, 2020 · 2 comments · Fixed by #16709
Assignees
@thomasjpfan
Labels
Bug

Comments

@TomDLT
Copy link
Member

TomDLT commented Mar 16, 2020 •
edited
Loading

The documentation of parametrize_with_checks claims that classes are accepted, but the function only accepts instances. The following example from the doc fails (launched with pytest):

from sklearn.utils.estimator_checks import parametrize_with_checks
from sklearn.linear_model import LogisticRegression
from sklearn.tree import DecisionTreeRegressor

@parametrize_with_checks([LogisticRegression, DecisionTreeRegressor])
def test_sklearn_compatible_estimator(estimator, check):
    check(estimator)
../../../../miniconda3/envs/py37/lib/python3.7/site-packages/pluggy/hooks.py:289: in __call__
    return self._hookexec(self, self.get_hookimpls(), kwargs)
../../../../miniconda3/envs/py37/lib/python3.7/site-packages/pluggy/manager.py:87: in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
../../../../miniconda3/envs/py37/lib/python3.7/site-packages/pluggy/manager.py:81: in <lambda>
    firstresult=hook.spec.opts.get("firstresult") if hook.spec else False,
../../../../miniconda3/envs/py37/lib/python3.7/site-packages/_pytest/python.py:225: in pytest_pycollect_makeitem
    res = list(collector._genfunctions(name, obj))
../../../../miniconda3/envs/py37/lib/python3.7/site-packages/_pytest/python.py:401: in _genfunctions
    self.ihook.pytest_generate_tests(metafunc=metafunc)
../../../../miniconda3/envs/py37/lib/python3.7/site-packages/pluggy/hooks.py:289: in __call__
    return self._hookexec(self, self.get_hookimpls(), kwargs)
../../../../miniconda3/envs/py37/lib/python3.7/site-packages/pluggy/manager.py:87: in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
../../../../miniconda3/envs/py37/lib/python3.7/site-packages/pluggy/manager.py:81: in <lambda>
    firstresult=hook.spec.opts.get("firstresult") if hook.spec else False,
../../../../miniconda3/envs/py37/lib/python3.7/site-packages/_pytest/python.py:128: in pytest_generate_tests
    metafunc.parametrize(*marker.args, **marker.kwargs)
../../../../miniconda3/envs/py37/lib/python3.7/site-packages/_pytest/python.py:993: in parametrize
    function_definition=self.definition,
../../../../miniconda3/envs/py37/lib/python3.7/site-packages/_pytest/mark/structures.py:122: in _for_parametrize
    parameters = cls._parse_parametrize_parameters(argvalues, force_tuple)
../../../../miniconda3/envs/py37/lib/python3.7/site-packages/_pytest/mark/structures.py:116: in _parse_parametrize_parameters
    ParameterSet.extract_from(x, force_tuple=force_tuple) for x in argvalues
../../../../miniconda3/envs/py37/lib/python3.7/site-packages/_pytest/mark/structures.py:116: in <listcomp>
    ParameterSet.extract_from(x, force_tuple=force_tuple) for x in argvalues
../scikit-learn/sklearn/utils/estimator_checks.py:403: in <genexpr>
    for estimator, check in checks_generator)
../scikit-learn/sklearn/utils/estimator_checks.py:364: in _mark_xfail_checks
    xfail_checks = _safe_tags(estimator, '_xfail_test')
../scikit-learn/sklearn/utils/estimator_checks.py:66: in _safe_tags
    return estimator._get_tags().get(key, _DEFAULT_TAGS[key])
E   TypeError: _get_tags() missing 1 required positional argument: 'self'
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 errors during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
========================================== 1 error in 0.39 seconds ===========================================

Fix A: (quickfix) update the docstring in parametrize_with_checks to allow only instances and not classes. Also update the example in "Rolling your own estimator".
Fix B: fix the behavior to accept classes, and add a test.
@TomDLT TomDLT added the Bug label Mar 16, 2020
@rth
Copy link
Member

rth commented Mar 16, 2020

Looks like a regression after #16510 indeed. Thanks for catching it @TomDLT . I agree we need a non regression test as well.

@thomasjpfan
Copy link
Member

take

@thomasjpfan thomasjpfan mentioned this issue Mar 16, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@thomasjpfan thomasjpfan

Labels
Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

BUG Ignores tags when estimator is a class in parametrize_with_checks thomasjpfan/scikit-learn
3 participants
@rth @thomasjpfan @TomDLT