8000 WIP Replace boston in estimator_checks.py by lucyleeow · Pull Request #16897 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

WIP Replace boston in estimator_checks.py #16897

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
wants to merge 1 commit into from
Closed

WIP Replace boston in estimator_checks.py #16897

wants to merge 1 commit into from

Conversation

lucyleeow
Copy link
Member

Reference Issues/PRs

Towards #16155

What does this implement/fix? Explain your changes.

Replace Boston dataset with diabetes dataset in sklearn/utils/estimator_checks.py

Any other comments?

@cmarmo
Copy link
Contributor
cmarmo commented Apr 14, 2020

Hi @lucyleeow , thanks for your PR! Tests are failing: this is probably due to the intensive use of check_estimator function in test_common.py. Could you please have a look? Feel free to ask if you have any questions. Cheers.

@lucyleeow
Copy link
Member Author
lucyleeow commented Apr 14, 2020 •
edited
Loading

Thanks @cmarmo - I was actually going to ask you for advice on this. I had a look and check_estimator is used in many tests. Further, some tests e.g., test_check_estimators_stacking_estimator are failing due to boston dataset being used to determine whether an estimator gets a poor_score tag (ref). E.g., failure from test_check_estimators_stacking_estimator:

         if not _safe_tags(regressor, "poor_score"):
>           assert regressor.score(X, y_) > 0.5
E           AssertionError

Not sure how estimators are tagged with poor_score so would love some help on this.
Also, this will end up as a big PR due to all the associated tests that need to be changed - not sure if there is a better alternative?

@lucyleeow
Copy link
Member Author

Also poor_score documentation says:

whether the estimator fails to provide a “reasonable” test-set score, which currently for regression is
an R2 of 0.5 on a subset of the boston housing dataset, and for classification an accuracy of 0.83 on
make_blobs(n_samples=300, random_state=0). These datasets and values are based on current
estimators in sklearn and might be replaced by something more systematic.

Maybe this is a good time replace the datasets with 'something more systematic'?

@adrinjalali
Copy link
Member

Maybe this is a good time replace the datasets with 'something more systematic'?

I'm not sure what systematic would mean. But I'd agree that apparently diabetes is not a good one for this particular task. Maybe one of the synthesized ones?

@cmarmo
Copy link
Contributor
cmarmo commented Apr 14, 2020

Also, this will end up as a big PR due to all the associated tests that need to be changed - not sure if there is a better alternative?

I think this is the only solution in order to keep consistency: maybe you can keep the _boston_subset function definition in a WIP stage of the PR, in order to identify which tests are failing because of their relation with this specific dataset (like the poor_score ones) and keep the issue about them open, while adapting to diabetes dataset all the other tests... do you think this is doable?

@lucyleeow
Copy link
Member Author

while adapting to diabetes dataset all the other tests... do you think this is doable?

I don't think this is possible because the tests use the check_estimator function which uses the boston dataset, so I can't change just the tests to use diabetes without changing the dataset used in check_estimator to be diabetes as well.

@adrinjalali how would you determine a good dataset for poor_score? One that gives similar results to boston or?

@adrinjalali
Copy link
Member

Our dev guide says:

poor_score (default=False)

whether the estimator fails to provide a “reasonable” test-set score, which currently for regression is an R2 of 0.5 on a subset of the boston housing dataset, and for classification an accuracy of 0.83 on make_blobs(n_samples=300, random_state=0). These datasets and values are based on current estimators in sklearn and might be replaced by something more systematic.

That to me means we need to find a dataset for which most models (i.e. everything in sklearn which doesn't have poor_score=True) would give a reasonably good result on the test set. Changing the dataset would mean changing this guide as well, but the closer the dataset and models' results to the existing one, the easier the transition.

@lucyleeow lucyleeow changed the title Replace boston in estimator_checks.py WIP Replace boston in estimator_checks.py Apr 24, 2020
@cmarmo cmarmo mentioned this pull request May 11, 2020
Closed
@lucyleeow
Copy link
Member Author

closed in preference of #17356

@lucyleeow lucyleeow closed this May 26, 2020
@lucyleeow lucyleeow deleted the est_checks branch May 26, 2020 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
module:utils
4695
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lucyleeow @cmarmo @adrinjalali
0