TST move check_n_features_in_after_fitting to common tests #29844
Conversation
| @pytest.mark.parametrize( | ||
| "estimator", _tested_estimators(), ids=_get_check_estimator_ids | ||
| ) | ||
| def test_check_n_features_in_after_fitting(estimator): |
There was a problem hiding this comment.
moved to common tests
| @@ -411,6 +411,14 @@ def predict_proba(self, X): | |||
| def __sklearn_tags__(self): | |||
| tags = super().__sklearn_tags__() | |||
| tags.classifier_tags.multi_label = True | |||
| tags.input_tags.pairwise = self.metric == "precomputed" | |||
There was a problem hiding this comment.
no idea why this was missing
sklearn/utils/estimator_checks.py
Outdated
| X = _enforce_estimator_tags_X(estimator, X) | ||
| y = _enforce_estimator_tags_y(estimator, y) |
There was a problem hiding this comment.
a bunch of these came up since now we're testing pairwise data classifieries in common tests which I guess we were not before.
sklearn/utils/estimator_checks.py
Outdated
| @@ -3925,6 +3960,27 @@ def check_n_features_in_after_fitting(name, estimator_orig): | |||
| ] | |||
| X_bad = X[:, [1]] | |||
|
|
|||
| err_msg = ( | |||
| "`{name}.{method}()` does not check for consistency between input number " | |||
There was a problem hiding this comment.
with these long comments, I want to make it very easy for people to fix issues and as as a result follow our API. I'll create more PRs extending our error messages with fix suggestions.
There was a problem hiding this comment.
Instead of using "\n" everywhere, can we use this pattern:
err_msg = textwrap.dedent("""\
This is
my
multiple
line error message
""")There was a problem hiding this comment.
I've used this format in other places I think, but I find it very ugly, since due to indentation it needs to be sth like:
def g():
err_msg = """This
is
my
message"""
print(err_msg)There was a problem hiding this comment.
With textwrap.dedent, you can indent without any ugly formatting.
There was a problem hiding this comment.
Done, but note that this adds a newline at the beginning and the end of the message (not important to me)
| @@ -753,10 +753,6 @@ def test_check_estimator(): | |||
| msg = "object has no attribute 'fit'" | |||
| with raises(AttributeError, match=msg): | |||
| check_estimator(BaseEstimator()) | |||
| # check that fit does input validation | |||
| msg = "Did not raise" | |||
| with raises(AssertionError, match=msg): | |||
There was a problem hiding this comment.
this is now redundant, tested in another test in the same file.
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
…arn#29844) Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
Move the test to common tests and include it in the API checks.