-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
documentation of attributes of HistGradientBoostingClassifier except … #16283
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
documentation of attributes of HistGradientBoostingClassifier except … #16283
Conversation
Thanks for the PR @judithabk6 However my impression is that all of these attributes should instead be private rather than public. WDYT @ogrisel? |
@@ -923,13 +923,24 @@ class HistGradientBoostingClassifier(BaseHistGradientBoosting, | |||
|
|||
Attributes | |||
---------- | |||
bin_mapper_ : BinMapper object |
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.
IMO, this could be private.
@@ -923,13 +923,24 @@ class HistGradientBoostingClassifier(BaseHistGradientBoosting, | |||
|
|||
Attributes | |||
---------- | |||
bin_mapper_ : BinMapper object | |||
Transformer that maps a dataset into integer-valued bins. | |||
do_early_stopping_ : bool |
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 could be public since it is derived from an input parameter.
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 very useful, also obsolete once #14516 is merged
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.
agreed this is obsolete now :)
Transformer that maps a dataset into integer-valued bins. | ||
do_early_stopping_ : bool | ||
Specifies if early stopping is used during the training. | ||
loss_ : callable |
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 could be public since it is derived from an input parameter.
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.
I disagree, the losses are all private objects
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.
The effective loss name could be stored as a public attribute.
Specifies if early stopping is used during the training. | ||
loss_ : callable | ||
Loss function used by the algorithm. | ||
n_features : int |
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 could be private
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.
Private for now and replaced by n_features_in_
later.
n_iter_ : int | ||
The number of estimators as selected by early stopping (if | ||
n_iter_no_change is not None). Otherwise it corresponds to max_iter. | ||
n_trees_per_iteration_ : int | ||
The number of tree that are built at each iteration. This is equal to 1 | ||
for binary classification, and to ``n_classes`` for multiclass | ||
classification. | ||
scorer_ : callable |
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 could be public
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.
I don't think this is a very useful public attribute. What are the benefit of exposing it?
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.
Indeed, we could also keep it private. The only other classes that have a public scorer_
attribute are the SearchCV classes and for those it's actually a list of scorer objects to deal with multi-metrics things.
Let's keep it private.
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.
Indeed we can probably close this PR and instead make those attributes private by moving the underscore at the start in the name of the attributes.
Transformer that maps a dataset into integer-valued bins. | ||
do_early_stopping_ : bool | ||
Specifies if early stopping is used during the training. | ||
loss_ : callable |
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.
The effective loss name could be stored as a public attribute.
Specifies if early stopping is used during the training. | ||
loss_ : callable | ||
Loss function used by the algorithm. | ||
n_features : int |
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.
Private for now and replaced by n_features_in_
later.
n_iter_ : int | ||
The number of estimators as selected by early stopping (if | ||
n_iter_no_change is not None). Otherwise it corresponds to max_iter. | ||
n_trees_per_iteration_ : int | ||
The number of tree that are built at each iteration. This is equal to 1 | ||
for binary classification, and to ``n_classes`` for multiclass | ||
classification. | ||
scorer_ : callable |
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.
Indeed, we could also keep it private. The only other classes that have a public scorer_
attribute are the SearchCV classes and for those it's actually a list of scorer objects to deal with multi-metrics things.
Let's keep it private.
@ogrisel, @glemaitre, @NicolasHug , is a consensus there about making all the attributes in this PR private? If so, @judithabk6 are you ok in proposing a new PR in that direction? Feel free to ask for clarifications if you need some. Thanks for your patience! |
Yes let's make everything private except for |
contributes to #14312
adds documentation for attributes of HistGradientBoostingClassifier, except classes_ (see #16277)
@NicolasHug attribute
bin_mapper_
is of private class_BinMapper
but is exposed, is that ok?