Skip to content

Conversation

lesteve
Copy link
Member

@lesteve lesteve commented Oct 24, 2023

I think the sphinx 6 pin is not necessary any more and the search seems to work locally with the latest sphinx release. Context for this #25504.

Also avoids confusing contributors like #27643.

The pandas pin was done in #27448 to avoid a warning. It should be fixed in seaborn 0.13 released September 29.

@github-actions
Copy link

✔️ Linting Passed

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

Generated for commit: 29185d4. Link to the linter CI: here

@lesteve
Copy link
Member Author

lesteve commented Oct 24, 2023

The doc build completed without error.

I looked at the generated doc vs the dev doc (the landing page, a few examples, a few user guide pages) and I didn't spot any obvious difference.

Search seems to work fine (this was why we pinned sphinx to 6.0 before): https://output.circle-artifacts.com/output/job/b88f0bab-bb68-4b74-aafc-4a6a674d8c9b/artifacts/0/doc/search.html?q=linear

There are no FutureWarning due to pandas and seaborn e.g. in this example

@lesteve lesteve added Quick Review For PRs that are quick to review No Changelog Needed labels Oct 24, 2023
Copy link
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

LGTM. We should probably revisit activating warning as error in the documentation as well. I need to comeback to #27454

Maybe @lesteve you would have some ideas to not catch some of useless warning there (I let you comment in #27454).

@betatim betatim merged commit 33d768c into scikit-learn:main Oct 24, 2023
@betatim
Copy link
Member

betatim commented Oct 24, 2023

Let's see what happens

@lesteve lesteve deleted the unpin-sphinx branch October 24, 2023 13:45
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Oct 31, 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
Labels
No Changelog Needed Quick Review For PRs that are quick to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants