Skip to content

MNT remove boston from the common test / estimator checks#17356

Merged
ogrisel merged 9 commits intoscikit-learn:masterfrom
glemaitre:is/remove_boston_common_test
Jun 8, 2020
Merged

MNT remove boston from the common test / estimator checks#17356
ogrisel merged 9 commits intoscikit-learn:masterfrom
glemaitre:is/remove_boston_common_test

Conversation

@glemaitre
Copy link
Copy Markdown
Member

closes #17182

Using make_regression instead of load_boston in the common test.

@lucyleeow
Copy link
Copy Markdown
Member

Towards #16155

@glemaitre glemaitre changed the title MNT remove boston from the common test MNT remove boston from the common test / estimator checks May 26, 2020
X, y = load_boston(return_X_y=True)
X, y = shuffle(X, y, random_state=0)
X, y = X[:n_samples], y[:n_samples]
def _regression_dataset(n_samples=200):
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.

Not a blocker. The first call to _regression_dataset will create a dataset with n_samples, and for all proceeding calls n_samples will do nothing.

At this point, I would just define REGRESSION_DATASET on top

REGRESSION_DATASET = make_regression(...)

And then have the checks do:

X, y = REGRESSION_DATASET

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

True

glemaitre and others added 2 commits May 27, 2020 18:38
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
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 @glemaitre

@cmarmo
Copy link
Copy Markdown
Contributor

cmarmo commented Jun 8, 2020

@adrinjalali , @amueller do you mind to have a look? Thanks!

But keep the lazy generation code.
Copy link
Copy Markdown
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

I went ahead and addressed #17356 (comment) by hardcoding the dataset size (but keeping the lazy dataset generation in a private helper function to avoid having too complex code being executed at module import time.

@ogrisel ogrisel merged commit 51df262 into scikit-learn:master Jun 8, 2020
viclafargue pushed a commit to viclafargue/scikit-learn that referenced this pull request Jun 26, 2020
jayzed82 pushed a commit to jayzed82/scikit-learn that referenced this pull request Oct 22, 2020
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.

New definition of poor_score Estimator tag

5 participants