-
-
Notifications
You must be signed in to change notification settings - Fork 25.5k
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
DOC revamp how to develop sklearn estimator page #30253
Conversation
Marked this as ready for review @glemaitre @adam2392 . I think these are the crucial areas which we better get in for the release since things have changed, and I'd be working on improving these / the rest of the document, and adding proper examples on how to write estimators in separate PRs. |
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 is already an improvement as is, so +1 from me.
doc/developers/develop.rst
Outdated
very few places, only in some meta-estimators, where the sub-estimator(s) argument is | ||
a required argument. | ||
|
||
The arguments should all correspond to hyperparameters describing the model or 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.
Not all arguments are hyparmeters or part of the optimization problem. For example CountVectorizer.input
is not in a search space for tuning a model.
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 was text from before, but I agree, changed now.
The way people should develop estimators has changed quite a lot in the past years, and we also have developed our developer API quite a bit in recent years.
This PR is an effort to make the guide on how to develop an estimator more up to date with what we expect users to do these days.