The Wayback Machine - https://web.archive.org/web/20210322131518/https://github.com/scikit-learn/scikit-learn/pull/19729
Skip to content
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 Fix load iris datasets #19729

Merged
merged 2 commits into from Mar 21, 2021
Merged

Conversation

@maliozer
Copy link
Contributor

@maliozer maliozer commented Mar 19, 2021

since the iris datasets is not defined above, the "name iris is not defined" error is thrown.

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

…s not defined" error is thrown.
Copy link
Member

@thomasjpfan thomasjpfan left a comment

Thank you for the PR @maliozer !

I would prefer this be defined during the "Using the Iris dataset" section in line 131:

    >>> from sklearn.datasets import load_iris
    >>> from sklearn import tree
    >>> iris = load_iris()
    >>> X, y = iris.data, iris.target
    >>> clf = tree.DecisionTreeClassifier()
    >>> clf = clf.fit(X, y)
@maliozer
Copy link
Contributor Author

@maliozer maliozer commented Mar 19, 2021

Thank you for the PR @maliozer !

I would prefer this be defined during the "Using the Iris dataset" section in line 131:

    >>> from sklearn.datasets import load_iris
    >>> from sklearn import tree
    >>> iris = load_iris()
    >>> X, y = iris.data, iris.target
    >>> clf = tree.DecisionTreeClassifier()
    >>> clf = clf.fit(X, y)

The reason I chose not to add it at the Using the Iris dataset section, is because it would be meaningless why the iris datasets is loaded twice until you don't apply the graphviz example. The above example demonstrates the return_X_y parameter usage. I tried to leave the example as it is.

@thomasjpfan
Copy link
Member

@thomasjpfan thomasjpfan commented Mar 19, 2021

For this specific case, I think it is nice to show the complete usage of the load_iris() Bunch object. The first part

    >>> from sklearn.datasets import load_iris
    >>> from sklearn import tree
    >>> iris = load_iris()
    >>> X, y = iris.data, iris.target
    >>> clf = tree.DecisionTreeClassifier()
    >>> clf = clf.fit(X, y)

shows how to extract the data and target from the iris Bunch object. Later, the graphviz example part shows how to get the feature names from the iris Bunch object.

Side note: Most of the user guide uses load_iris(return_X_y=True), so having an example with the default return_X_y=False would be beneficial.

Copy link
Member

@thomasjpfan thomasjpfan left a comment

LGTM

@thomasjpfan thomasjpfan merged commit 1db2681 into scikit-learn:main Mar 21, 2021
26 checks passed
26 checks passed
check
Details
triage
Details
Check build trigger
Details
Build wheel for cp${{ matrix.python }}-${{ matrix.platform_id }}-${{ matrix.manylinux_image }}
Details
Source distribution
Details
Upload to Anaconda
Details
LGTM analysis: C/C++ No code changes detected
Details
LGTM analysis: JavaScript No code changes detected
Details
LGTM analysis: Python No code changes detected
Details
ci/circleci: deploy Your tests passed on CircleCI!
Details
ci/circleci: doc Your tests passed on CircleCI!
Details
ci/circleci: doc artifact Link to 0/doc/_changed.html
Details
ci/circleci: doc-min-dependencies Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
scikit-learn.scikit-learn Build #20210319.33 succeeded
Details
scikit-learn.scikit-learn (Get Git Commit) Get Git Commit succeeded
Details
scikit-learn.scikit-learn (Linting) Linting succeeded
Details
scikit-learn.scikit-learn (Linux py36_conda_openblas) Linux py36_conda_openblas succeeded
Details
scikit-learn.scikit-learn (Linux py36_ubuntu_atlas) Linux py36_ubuntu_atlas succeeded
Details
scikit-learn.scikit-learn (Linux pylatest_pip_openblas_pandas) Linux pylatest_pip_openblas_pandas succeeded
Details
scikit-learn.scikit-learn (Linux32 py36_ubuntu_atlas_32bit) Linux32 py36_ubuntu_atlas_32bit succeeded
Details
scikit-learn.scikit-learn (Linux_Runs pylatest_conda_mkl) Linux_Runs pylatest_conda_mkl succeeded
Details
scikit-learn.scikit-learn (Windows py36_pip_openblas_32bit) Windows py36_pip_openblas_32bit succeeded
Details
scikit-learn.scikit-learn (Windows py37_conda_mkl) Windows py37_conda_mkl succeeded
Details
scikit-learn.scikit-learn (macOS pylatest_conda_forge_mkl) macOS pylatest_conda_forge_mkl succeeded
Details
scikit-learn.scikit-learn (macOS pylatest_conda_mkl_no_openmp) macOS pylatest_conda_mkl_no_openmp succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants