Skip to content
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.

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

allow testing meta_estimators from outside scikit-learn #6423

Closed
wants to merge 1 commit into from
Closed

allow testing meta_estimators from outside scikit-learn #6423

wants to merge 1 commit into from

Conversation

niedakh
Copy link
Contributor

@niedakh niedakh commented Feb 22, 2016

Hi,

I'm working on scikit-multilearn and I was wondering if you could help me out a little so that I can use scikit's check_estimator on multi-label classifiers that are not in scikit's repo.

The code adds a possibility to treat an external classifier as what scikit-learn would call a meta estimator.

@amueller
Copy link
Member

amueller commented Mar 3, 2016

We should fix this, but maybe not the way you did.
The hard-coded constants are certainly bad (and there are several of them).

I think a meta-estimator should identify itself (by the mixin?), so that the tests know.
We currently have estimator_type but that was likely a bad decision. Maybe that should be a set or a dict? Currently it's a string, and meta-estimator have the estimator type of the thing they wrap (in the case of pipelines at least - your classifier would always be a classifier).

@niedakh
Copy link
Contributor Author

niedakh commented Mar 4, 2016

So a MetaEstimatorMixin inheriting from a ClassifierMixin that adds a function is_metaestimator() {return True} is a good choice?

@amueller
Copy link
Member

amueller commented Oct 8, 2016

Sorry for the silence. The solution is this: #6599
So yes, probably a base-class and add estimator_type='meta-estimator' or something like that.
Unfortunately, that will not really run any tests, only the very weak one we currently run. We should probably enable all test to be run by meta-estimators by using a factory.

@amueller
Copy link
Member

You can now pass instances: #9019 that should solve your problem. #8022 should further help by allowing instantiation of meta-estimator classes within the common tests. Any feedback welcome!

@amueller amueller closed this Jun 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants