-
-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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
[MRG] Added doc on how to override estimator tags #13550
[MRG] Added doc on how to override estimator tags #13550
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Another thing we should do is reference relevant tags in error messages raised by estimator checks.
doc/whats_new/v0.21.rst
Outdated
@@ -553,3 +553,8 @@ These changes mostly affect library developers. | |||
when `fit` is called twice with the same data, the ouput of | |||
`predict`, `predict_proba`, `transform`, and `decision_function` does not | |||
change. :issue:`12328` by :user:`Nicolas Hug <NicolasHug>` | |||
|
|||
- Add estimators tags: these are annotations of estimators that allow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this belongs in "Multiple modules" and the comment here should be that some checks will now be run selectively depending on the estimator's tags
doc/whats_new/v0.21.rst
Outdated
|
||
- Add estimators tags: these are annotations of estimators that allow | ||
programmatic inspection of their capabilities, such as sparse matrix | ||
support, supported output types and supported methods. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reference the user guie
Thanks for the review Joel. Should this be labeled as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
I agree that error messages could also be improved but maybe in a separate PR?
doc/whats_new/v0.21.rst
Outdated
- Add estimators tags: these are annotations of estimators that allow | ||
programmatic inspection of their capabilities, such as sparse matrix | ||
support, supported output types and supported methods. The estimators checks | ||
are now run selectively, depending on the estimator's tags. Read more in the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may sounds a bit confusing, as estimator checks were always run selectively. Maybe,
Estimator tags also determine tests that are run on an estimator when `check_estimator` is called.
doc/whats_new/v0.21.rst
Outdated
@@ -542,6 +542,12 @@ Multiple modules | |||
- |Efficiency| Memory copies are avoided when casting arrays to a different | |||
dtype in multiple estimators. :issue:`11973` by :user:`Roman Yurchak | |||
<rth>`. | |||
- Add estimators tags: these are annotations of estimators that allow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- |MajorFeature|
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could just be feature in terms of user impact. It's a major achievement on our part
Reference Issues/PRs
Closes #13374
What does this implement/fix? Explain your changes.
_more_tags
wasn't mentioned anywhere)