Skip to content

DOC Update link to best sphinx version for doc build #26626

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 3 commits into from
Jun 21, 2023

Conversation

lucyleeow
Copy link
Member

Reference Issues/PRs

Follow on from #26610, relates to #26598

What does this implement/fix? Explain your changes.

As suggested here: #26598 (comment)
The current search returns results in 2 files, one of them giving the min sphinx version, which we don't want and this could be confusing. New link is this.

Any other comments?

@github-actions
Copy link

github-actions bot commented Jun 20, 2023

Linting Passed

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

@Charlie-XIAO
Copy link
Contributor

Charlie-XIAO commented Jun 21, 2023

@lucyleeow Sorry to chip in, but I just noticed that scikit-learn tests both with doc_min_dependencies_linux-64_conda.lock and doc_linux-64_conda.lock (if I were not mistaken).

image

Therefore would it be better to leave it as is, especially since it is pretty clear which is the best version and which is the min version (either from the filename or from the comment)?

image

@glemaitre
Copy link
Member

I would be inclined toward only proposing a single solution. Advanced programmers will always get their heads around that stuff. I am worried about beginners and there is little value to offer several options and we should only mention the latest supported version (which I hope would be the latest release apart from when the CI is broken.

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.

I am happy with this. LGTM

@thomasjpfan thomasjpfan merged commit 4a8b4f9 into scikit-learn:main Jun 21, 2023
@lucyleeow lucyleeow deleted the doc_sphinxlink branch June 22, 2023 01:34
Vincent-Maladiere pushed a commit to Vincent-Maladiere/scikit-learn that referenced this pull request Jun 22, 2023
jeremiedbb pushed a commit to jeremiedbb/scikit-learn that referenced this pull request Jun 29, 2023
REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023
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.

4 participants