Skip to content

Conversation

cmarmo
Copy link
Contributor

@cmarmo cmarmo commented Jun 23, 2020

Reference Issues/PRs

Closes #16283
Related to #14312

What does this implement/fix? Explain your changes.

Documents missing attribute do_early_stopping, makes private some other previously undocumented attributes (see comment)

@cmarmo
Copy link
Contributor Author

cmarmo commented Jun 23, 2020

Closing in favor of #17677
#17677 is about Regressor not Classifier.

@cmarmo cmarmo closed this Jun 23, 2020
@cmarmo cmarmo reopened this Jun 23, 2020
Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking care of this @cmarmo , some minor comments

@@ -1109,6 +1111,8 @@ class HistGradientBoostingClassifier(BaseHistGradientBoosting,
----------
classes_ : array, shape = (n_classes,)
Class labels.
do_early_stopping_ : bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll also need this for the regressor

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, @simonamaggio is taking care of that in #17677.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it'd be easier to have everything in the same PR so that we can ensure consistency, but OK

@cmarmo cmarmo added the Superseded PR has been replace by a newer PR label Jun 24, 2020
@cmarmo
Copy link
Contributor Author

cmarmo commented Jun 24, 2020

Labeled as 'superseded' @simonamaggio will take care of the changes for HistGradientBoostingClassifier and HistGradientBoostingRegressor` following @NicolasHug suggestions.

@glemaitre glemaitre merged commit 5ea5da4 into scikit-learn:master Jun 24, 2020
@glemaitre
Copy link
Member

Thanks @cmarmo

@cmarmo
Copy link
Contributor Author

cmarmo commented Jun 24, 2020

@glemaitre wrong PR... this is superseded by #17677

@glemaitre
Copy link
Member

Uhm we need both. One is the classifier, the other one is the regressor.

rubywerman pushed a commit to MLH-Fellowship/scikit-learn that referenced this pull request Jun 24, 2020
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Jul 17, 2020
@cmarmo cmarmo deleted the histgradboostclass_attributes branch August 8, 2020 11:04
jayzed82 pushed a commit to jayzed82/scikit-learn that referenced this pull request Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module:ensemble Superseded PR has been replace by a newer PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants