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

UPGRADE: Sphinx 6.x. #106

Merged
merged 4 commits into from Apr 6, 2023
Merged

UPGRADE: Sphinx 6.x. #106

merged 4 commits into from Apr 6, 2023

Conversation

marxin
Copy link
Contributor

@marxin marxin commented Nov 7, 2022

There should not be any fundamental problem with supporting Sphinx 6.x.

To do

  • New myst-parser is released, which will let us test on Sphinx 6 here.
  • See if tests are happy or anything broken in the output
  • If not, we can merge

Closes #118

@welcome
Copy link

welcome bot commented Nov 7, 2022

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.

Welcome to the EBP community! 🎉

@jpmckinney
Copy link
Contributor

Seeing as Sphinx 6 isn't finalized or released yet, it seems prudent to wait for a release before changing this. Major Sphinx releases have a tendency to upgrade docutils, which has a tendency to break things.

@marxin
Copy link
Contributor Author

marxin commented Dec 8, 2022

Works for me.

@akaszynski
Copy link

Would be great to add support for this as this is blocking PyVista in pyvista/pyvista#3762.

@benjaoming
Copy link

Seeing that docutils 0.18 and 0.19 have already been released for a while, that should not be a source of issues.

Sphinx 6 removed jQuery but I can't find anywhere that sphinx-design uses this?

I don't know anything else that could raise concerns here. The release notes are available here: https://www.sphinx-doc.org/en/master/changes.html

srideep3
srideep3 previously approved these changes Jan 6, 2023
@choldgraf
Copy link
Member

I'm +1 on this, but we'll need to bump the versions to latest of all of the other dependencies in tests that in EBP. They tend to cap the upper version of Sphinx in each one, so I suspect that the version of Sphinx we test here is going to be 5 even if we raise the version of this package to 6.

@benjaoming
Copy link

@choldgraf you're right!

Collecting sphinx<7,>=4
  Downloading sphinx-5.3.0-py3-none-any.whl (3.2 MB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 3.2/3.2 MB 28.3 MB/s eta 0:00:00

Seems we need a new release of myst-parser to come out, Sphinx 6 was added yesterday: executablebooks/MyST-Parser#664

@codecov-commenter
Copy link

codecov-commenter commented Jan 16, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.01 🎉

Comparison is base (6df4751) 90.02% compared to head (c4a2fb4) 90.03%.

❗ Current head c4a2fb4 differs from pull request most recent head 93ac84d. Consider uploading reports for the commit 93ac84d to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #106      +/-   ##
==========================================
+ Coverage   90.02%   90.03%   +0.01%     
==========================================
  Files          11       10       -1     
  Lines         942      933       -9     
==========================================
- Hits          848      840       -8     
+ Misses         94       93       -1     
Flag Coverage Δ
pytests 90.03% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@spoorcc
Copy link

spoorcc commented Mar 1, 2023

There is a new MyST-Parser release available:
executablebooks/MyST-Parser#710 (comment)

@nicoa
Copy link

nicoa commented Mar 15, 2023

Hey, thanks for the work on this! Linking #118 for reference here, as it is closed by this PR.

@LecrisUT
Copy link

LecrisUT commented Apr 6, 2023

@chrisjsewell or any other collaborator. Can someone trigger the tests and/or review this?

Updating to myst 1.0 so that we no longer pin Sphinx
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
Copy link
Member

@choldgraf choldgraf left a comment

Choose a reason for hiding this comment

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

This seems reasonable to me! I updated the MyST Parser as well because that was pinning Sphinx at myst-parser 0.18. I see that tests are happy (the 3.8 failure is because of a codecov upload error, not the test suite), so I'll merge this one!

@choldgraf choldgraf changed the title Support Sphinx 6.x. UPGRADE: Sphinx 6.x. Apr 6, 2023
@choldgraf choldgraf merged commit f9a6931 into executablebooks:main Apr 6, 2023
12 of 14 checks passed
@welcome
Copy link

welcome bot commented Apr 6, 2023

Congrats on your first merged pull request in this project! 🎉
congrats

Thank you for contributing, we are very proud of you! ❤️

@LecrisUT
Copy link

LecrisUT commented Apr 6, 2023

@choldgraf Thank you very much 🎉. Seems that pre-commit CI needs some upgrading though?

@marxin marxin deleted the sphinx-6 branch April 7, 2023 07:10
@LecrisUT
Copy link

Any remaining fixes until version bump?

@choldgraf
Copy link
Member

choldgraf commented Apr 13, 2023

Nothing blocking, just hours in the day. If others are interested in helping to maintain, merge PRs, cut releases, etc, please let me know!

Here's a PR to cut a new release:

If the tests are happy then I'll merge and release on github 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support sphinx 6
10 participants