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

[FEA] Add an _estimator_type attribute to estimators #2393

Closed
beckernick opened this issue Jun 11, 2020 · 2 comments · Fixed by #2411
Closed

[FEA] Add an _estimator_type attribute to estimators #2393

beckernick opened this issue Jun 11, 2020 · 2 comments · Fixed by #2411
Assignees
Labels
Cython / Python Cython or Python issue feature request New feature or request

Comments

@beckernick
Copy link
Member

beckernick commented Jun 11, 2020

In scikit-learn, estimators have an _estimator_type attribute that indicates whether they are a classifier, regressor, or clusterer, etc.. This attribute is necessary for some scikit-learn API contracts, particularly around cross validation and meta-modeling. See here for the documentation.

For better compatibility with scikit-learn and higher-level tools enforcing the same contract, it would be useful if cuML estimators had this attribute as well. scikit-learn implements these in their Mixin classes. Because not all cuML estimators inherit from Mixins currently, perhaps these could be added in the estimator classes themselves as a start, and then handled via Mixin classes if/when additional estimators inherit from them.

Maybe something like:

class LogisticRegression(Base):
    def __init__(self, penalty='l2', tol=1e-4, C=1.0, fit_intercept=True,
                 class_weight=None, max_iter=1000, linesearch_max_iter=50,
                 verbose=False, l1_ratio=None, solver='qn',
                 handle=None):

        super(LogisticRegression, self).__init__(handle=handle,
                                                 verbose=verbose)
        ...
        self.C = C
        ...
       self._estimator_type = "classifier"
        ...
@beckernick beckernick added feature request New feature or request Cython / Python Cython or Python issue labels Jun 11, 2020
@beckernick
Copy link
Member Author

beckernick commented Jun 11, 2020

Note that the Mixin classes already implement this attribute, so another approach would be to update estimators to inherit from the existing Mixins.

Perhaps these Mixins could be moved out of the metrics.base namespace and into the general common.base namespace? This would be consistent with sklearn

"""Mixin class for classifier estimators in"""
_estimator_type = "classifier"

class RegressorMixin:
"""Mixin class for regression estimators in"""
_estimator_type = "regressor"

@beckernick
Copy link
Member Author

After discussing with @dantegd , this seems like a good opportunity to update the existing Mixin classes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cython / Python Cython or Python issue feature request New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant