Skip to content

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

Merged
thomasjpfan merged 3 commits intoscikit-learn:mainfrom
lucyleeow:doc_sphinxlink
Jun 21, 2023
Merged

DOC Update link to best sphinx version for doc build#26626
thomasjpfan merged 3 commits intoscikit-learn:mainfrom
lucyleeow:doc_sphinxlink

Conversation

@lucyleeow
Copy link
Copy Markdown
Member

@lucyleeow lucyleeow commented Jun 20, 2023

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
Copy Markdown

github-actions bot commented Jun 20, 2023 •
edited
Loading

Linting Passed

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

@Charlie-XIAO
Copy link
Copy Markdown
Contributor

Charlie-XIAO commented Jun 21, 2023 •
edited
Loading

@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
Copy Markdown
Member

glemaitre commented Jun 21, 2023

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.

thomasjpfan
thomasjpfan approved these changes Jun 21, 2023
View reviewed changes
Copy link
Copy Markdown
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
@lucyleeow @Vincent-Maladiere
DOC Update link to best sphinx version for doc build (scikit-learn#26626
8e94b7f 
)
jeremiedbb pushed a commit to jeremiedbb/scikit-learn that referenced this pull request Jun 29, 2023
@lucyleeow @jeremiedbb
DOC Update link to best sphinx version for doc build (scikit-learn#26626
2f49260 
)
jeremiedbb pushed a commit that referenced this pull request Jun 29, 2023
@lucyleeow @jeremiedbb
DOC Update link to best sphinx version for doc build (#26626)
c21aa59
REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023
@lucyleeow @REDVM
DOC Update link to best sphinx version for doc build (scikit-learn#26626
a9ea388 
)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thomasjpfan thomasjpfan thomasjpfan approved these changes

@glemaitre glemaitre glemaitre approved these changes

Assignees

No one assigned

Labels

Documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lucyleeow @Charlie-XIAO @glemaitre @thomasjpfan