[WIP] Use dataset factories for estimator checks #17544
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…into dataset_factory # Conflicts: # sklearn/utils/estimator_checks.py
rth
reviewed
Jun 9, 2020
There was a problem hiding this comment.
Thanks for working on this @rtavenar ! Yeah I was thinking something along these lines. Maybe,
def default_dataset_factory(n_samples=50, kind='random', **tags):
...and then used as
dataset_factory(kind='random', **estimator.get_tags())One thing is we would need something like kind parameter because I suspect we would still need different types of datasets even e.g. for dense data. WDYT?
|
BTW, there might be some intersection with generating datasets for benchmarks in #17026 cc @jeremiedbb Edit: pinging a potential reviews just to get some general feedback before implementing this further. |
|
Yes, you're right that tags already contain a lot of useful information, I will use those when possible. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reference Issues/PRs
This is a follow-up of #14057
What does this implement/fix? Explain your changes.
The idea here is to provide a dataset factory to the estimator checks. This would allow downstream packages to define their own dataset factory to check their estimators on the specific kind of data they expect as input.
Any other comments?
This first commit is a small step towards implementing @rth 's suggestion, just to make sure that I did not misunderstand his suggestion.
The idea would be to use the dataset factory for all
check_*functions (or at least for as many functions as possible).