Skip to content

WIP add score_samples method in base search CV and add tests#16275

Closed
maskani-moh wants to merge 3 commits intoscikit-learn:masterfrom
maskani-moh:search-cv-score-samples-support
Closed

WIP add score_samples method in base search CV and add tests#16275
maskani-moh wants to merge 3 commits intoscikit-learn:masterfrom
maskani-moh:search-cv-score-samples-support

Conversation

@maskani-moh
Copy link
Copy Markdown
Contributor

Reference Issues/PRs

Fixes #12542

What does this implement/fix? Explain your changes.

With the changes we'll be now able to call score_samples on the estimator with the best found parameters in *SearchCV instances.

@jnothman
Copy link
Copy Markdown
Member

Please check the linter errors

Copy link
Copy Markdown
Contributor Author

@maskani-moh maskani-moh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Help needed on the test here, as it fails because the message error is not caught. The test should check that it actually errors out.

# the method `score_samples`
err_msg = "AttributeError: 'DecisionTreeClassifier' object has " \
"no attribute 'score_samples'"
assert_raise_message(AttributeError, err_msg, clf.score_samples, X)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that the error here is not caught and the test fails. How can I catch the error?
AttributeError: 'DecisionTreeClassifier' object has no attribute 'score_samples'"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try:

    err_msg = ("'DecisionTreeClassifier' object has no attribute "
               "'score_samples'")
    with pytest.raises(AttributeError, match=err_msg):
        clf.score_samples(X)

Copy link
Copy Markdown
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR @maskani-moh !

Parameters
----------
X : iterable
Data to predict on. Must fulfill input requirements of first step
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about:

Must fulfill input requirements of the underlying estimator

# the method `score_samples`
err_msg = "AttributeError: 'DecisionTreeClassifier' object has " \
"no attribute 'score_samples'"
assert_raise_message(AttributeError, err_msg, clf.score_samples, X)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try:

    err_msg = ("'DecisionTreeClassifier' object has no attribute "
               "'score_samples'")
    with pytest.raises(AttributeError, match=err_msg):
        clf.score_samples(X)



@pytest.mark.filterwarnings("ignore:The parameter 'iid' is deprecated") # 0.24
@pytest.mark.parametrize('search_cv', [RandomizedSearchCV, GridSearchCV])
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can create the objects here:

@pytest.mark.parametrize('search_cv', 
    [RandomizedSearchCV(estimator=..., param_distributions=...)

And we will not need the if statement in the test.


@pytest.mark.filterwarnings("ignore:The parameter 'iid' is deprecated") # 0.24
@pytest.mark.parametrize('search_cv', [RandomizedSearchCV, GridSearchCV])
def test_search_cv_score_samples_method(search_cv):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can create the objects here:

@pytest.mark.parametrize('search_cv', 
    [RandomizedSearchCV(estimator=..., param_distributions=...)

And we will not need the if statement in the test.

@maskani-moh
Copy link
Copy Markdown
Contributor Author

Thanks @thomasjpfan for your feedback!
I will make the changes on the PR asap

@amueller
Copy link
Copy Markdown
Member

are you still working on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a score_samples method to Pipeline and *SearchCV

5 participants