8000 DOC highlights: stacking estimators by adrinjalali · Pull Request #15414 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

DOC highlights: stacking estimators #15414

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

Merged
merged 6 commits into from
Oct 31, 2019

Conversation

adrinjalali
Copy link
Member

Towards #15152

This is the example we have already in its docstring.

@adrinjalali adrinjalali added this to the 0.22 milestone Oct 31, 2019
@adrinjalali
Copy link
Member Author

@thebooort sorry I saw your message too late :/

from sklearn.ensemble import StackingClassifier
from sklearn.model_selection import train_test_split

X, y = load_iris(return_X_y=True)
Copy link
Member

Choose a reason for hiding this comment

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

For the docstring, it was enough but do you think that in the highlight we should use another dataset than iris?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the highlights should be as easy as the docstrings, and the user guide is where the examples should be maybe more meaningful. We can link to the userguide for more info though.

Copy link
Member

Choose a reason for hiding this comment

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

OK I see LGTM then

# allows to use the strength of each individual estimator by using their output
# as input of a final estimator.
# Note that ``estimators_`` are fitted on the full ``X`` while
# ``final_estimator``_ is trained using cross-validated predictions of the
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# ``final_estimator``_ is trained using cross-validated predictions of the
# ``final_estimator_`` is trained using cross-validated predictions of the

############################################################################
# Stacking Classifier (and Regressor)
# -----------------------------------
# These new estimators are a stack of estimators with a final classifier or
Copy link
Member

Choose a reason for hiding this comment

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

Remove the first line and link to estimators?

StackingClassifer and StackingRegressor implements stacked generalization, which consist of stacking the output...

Copy link
Member
@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

The

.. currentmodule:: sklearn

On the top allows us to not need the sklearn. part in references in this file.

@@ -12,7 +12,7 @@

To install the latest version (with pip)::

pip install -U scikit-learn --upgrade
pip install -U scikit-learn
Copy link
Member

Choose a reason for hiding this comment

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

why?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm confused by this. As I understand, -U is the same as --upgrade, isn't it?

Copy link
Member

Choose a reason for hiding this comment

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

lol I thought it was for --user (which I've never ever used)

but you're right, let's keep --upgrade and remove -U then

@adrinjalali adrinjalali merged commit be027e0 into scikit-learn:master Oct 31, 2019
@adrinjalali adrinjalali deleted the highlights/1 branch October 31, 2019 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0