You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We currently partially support classes on check_estimator() or parametrize_with_checks(). However, they don't work in general when the class requires some parameter with no default (except the hard-coded special cases of 'estimator' and 'base_estimator').
As far as I understand, the only thing that class support adds is the additional check_parameters_default_constructible check.
Class support leads to complicated logic for example with the xfail_checks tag where we need to check if we can first construct and instance, and if we can't, we just silently ignore the tag.
Couldn't we just let check_estimator() and parametrize_with_checks work only on instances, and publicly expose check_parameters_default_constructible separately? I think it would make the test suite logic significantly simpler. In terms of usage, it's a very mild inconvenience.
+1. We can still introduce the deprecation warning in this release.
Deprecating class shouldn't be an issue for things like #11324 right? cc @jnothman
In any case this would go in the direction that instead of default __init__ parameters, testing different combinations of init parameter is probably recommended (at least for solvers etc).
We currently partially support classes on
check_estimator()
orparametrize_with_checks()
. However, they don't work in general when the class requires some parameter with no default (except the hard-coded special cases of 'estimator' and 'base_estimator').As far as I understand, the only thing that class support adds is the additional
check_parameters_default_constructible
check.Class support leads to complicated logic for example with the
xfail_checks
tag where we need to check if we can first construct and instance, and if we can't, we just silently ignore the tag.Couldn't we just let
check_estimator()
andparametrize_with_checks
work only on instances, and publicly exposecheck_parameters_default_constructible
separately? I think it would make the test suite logic significantly simpler. In terms of usage, it's a very mild inconvenience.CC @rth @thomasjpfan
The text was updated successfully, but these errors were encountered: