Skip to content

DOC remove tutorials #29104

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 13 commits into from
Jun 5, 2024
Merged

DOC remove tutorials #29104

merged 13 commits into from
Jun 5, 2024

Conversation

adrinjalali
Copy link
Member

doc/tutorials is not maintained and in some cases the scripts don't even run.

We can open issues if there's something we'd like to add from here to the documentation, but it doesn't really make sense to keep them around.

Copy link

github-actions bot commented May 24, 2024

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: ef4b210. Link to the linter CI: here

@lorentzenchr
Copy link
Member

+1 for removal.

The only thing that we might either keep or redo is https://scikit-learn.org/stable/tutorial/machine_learning_map/index.html. That was quite famous in the past.

Copy link
Member

@lorentzenchr lorentzenchr left a comment

Choose a reason for hiding this comment

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

LGTM

I was wondering about the out-datedness of the tutorial several times.

Copy link
Contributor

@Charlie-XIAO Charlie-XIAO left a comment

Choose a reason for hiding this comment

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

Also +1 for removing the outdated tutorial docs. LGTM with the following resolved:

adrinjalali and others added 2 commits May 27, 2024 15:15
Co-authored-by: Yao Xiao <108576690+Charlie-XIAO@users.noreply.github.com>
Co-authored-by: Yao Xiao <108576690+Charlie-XIAO@users.noreply.github.com>
@betatim
Copy link
Member

betatim commented May 27, 2024

Naive question: what is outdated about the tutorials? I had a quick look at the supervised classification, unsupervised classification, model selection and clustering pages. While they seem very basic, I couldn't immediately spot anything that was wrong about them. They can/are useful to introduce people to ideas and concepts that they (hopefully) already know but using the "scikit-learn lingo/jargon".

One of the frequently mentioned "best things about scikit-learn" are the documentation and tutorials. So I think we need more than "they are outdated" and "doesn't make sense to keep them". Basically, some slightly more detailed explanation of why getting rid of them is a useful move to make. (So far I have no opinion either way)

@adrinjalali
Copy link
Member Author

They have not been maintained, to the point that sometimes they wouldn't even run. They're not a part of the CI, and they're not even formatted.

We never add content to them, and whenever we want to add a tutorial like thing, it's either an example or a user guide. Most "tutorials" people refer to are from our example gallery.

As I said here, happy to have separate issues and add basic info to our documentation which were in the tutorials.

But as the tutorial folder itself, it's not something we have looked at in a couple of years. And since these days we would add tutorial like material as an example, better to move them there if somebody feels they should be somewhere.

Copy link
Contributor

@OmarManzoor OmarManzoor left a comment

Choose a reason for hiding this comment

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

@adrinjalali I think we should be able to merge this once the conflict is resolved.

@Charlie-XIAO
Copy link
Contributor

Charlie-XIAO commented May 30, 2024

BTW I wish doc/tutorial/machine_learning_map/README.md can be kept in some way. We don’t frequently update the machine learning map so I think it’s nice to keep that note somewhere in case we forget how to do it in the future.

@adrinjalali
Copy link
Member Author

Added #29167 , and a redirect for the old ml map page (need to check if it works).

After checking, this should be ready to merge.

@lorentzenchr
Copy link
Member

@OmarManzoor @Charlie-XIAO I let you merge in case you have further review comments.

@adrinjalali
Copy link
Member Author

Merging since no objections and two approvals.

@adrinjalali adrinjalali merged commit 65c00c7 into scikit-learn:main Jun 5, 2024
30 checks passed
@adrinjalali adrinjalali deleted the tutorials branch June 5, 2024 10:01
jeremiedbb pushed a commit to jeremiedbb/scikit-learn that referenced this pull request Jul 2, 2024
Co-authored-by: Yao Xiao <108576690+Charlie-XIAO@users.noreply.github.com>
@jeremiedbb jeremiedbb mentioned this pull request Jul 2, 2024
11 tasks
jeremiedbb pushed a commit to jeremiedbb/scikit-learn that referenced this pull request Jul 2, 2024
Co-authored-by: Yao Xiao <108576690+Charlie-XIAO@users.noreply.github.com>
jeremiedbb pushed a commit that referenced this pull request Jul 2, 2024
Co-authored-by: Yao Xiao <108576690+Charlie-XIAO@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants