-
Notifications
You must be signed in to change notification settings - Fork 200
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
Remove JQuery and update versions #668
Conversation
@AakashGfude could you have a look at this one? |
Also, may I ask you to build a new rc version when this is merged? I will try it out in our repo and will tell you if something goes wrong. |
Hmm if you need this quickly it may need to be split into multiple PRs. This currently builds off of main for the pydata theme, so we'd need to wait for a release there. |
No pressure here, we're not in a hurry at all :) |
@mathbunnyru you can also install from the branch directly if you want to experiment i think! I always end up googling how though, but i think @choldgraf may have a blogpost about it? 😅🥰 |
I actually tried it once locally and it didn't work for me on my m2 mac. I will try once again, when I have time :) |
A few more differences I found with this PR (I compared it with the previous one): It now adds Code blocks highlighting looks different. This page is not rendered properly at all - images and dataframes are missing, interactive part is also gone. |
Is the title purposefully made visible? EDIT: Ok, got it, based on this issue: #672 |
Something crazy happened in But the same problem is in the production page as well: https://sphinx-book-theme.readthedocs.io/en/latest/reference/extensions.html . I reckon the sphinx-tabs extension is interfering with something. |
pyproject.toml
Outdated
@@ -23,6 +23,7 @@ readme = "README.md" | |||
requires-python = ">=3.7" | |||
dependencies = [ | |||
"sphinx>=4,<7", | |||
"docutils==0.17.1", # docutils 0.18, 0.19 need a patch fix https://sourceforge.net/p/docutils/patches/195/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love comments like these as they make us able to unpin and upgrade things.
I see sphinx-tabs was upper-bounded as well, can you add a comment be added about that also?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @AakashGfude for working this!!!!
Co-authored-by: Chris Holdgraf <choldgraf@gmail.com>
Another idea we had re: pinning versions was the following, curious what you think as well @consideRatio :
I also opened an issue in sphinxcontrib-bibtex to see if they'd like the message in there, but it might be easier to do it here in the short term: |
I think its good if a package can declare what versions won't work in requirements directly, and ideally while avoiding placing upper bound that needs to be removed in the future. But, if one doesn't know in what release something is patched, one kind of have to. If we know it will be resolved in 0.20 but not in 0.18 and 0.19 etc, then I think this is what to go for if this works I didn't understand the options about providing messages @choldgraf, but I'd like to avoid digging into this further since I have many balls in the air atm. |
this semicolon is coming from pydata: https://github.com/pydata/pydata-sphinx-theme/blob/main/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/copyright.html#L7. So, we need to fix it there. |
The outputs are working now. |
Thanks @AakashGfude for your work on this. I did a review of the rendered docs pages as a second check and here are a few minor things I noticed:
|
@mmcky @choldgraf this build with sphinx5 and docutils0.17 works well with pydata-sphinx-theme==0.12.0 as well (the latest stable release). Should we just use that? It does not have issues like "Extra ; after copyright symbol" etc as well. |
@AakashGfude I'd suggest using the pydata
Do those issues show up there as well? |
FYI - the pydata-sphinx-theme has a release candidate out! Check it out here: @AakashGfude I see what you were saying about the missing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK following @AakashGfude's advice I think we should just merge this one, since it is already complex enough and make a clear improvement over our current situation. We will still need to update this theme again when the pydata theme releases once more, but I think that will be easier in a targeted PR.
I think there may also be some lingering UX issues that @mmcky , but could we confirm that they are present on our docs, and open issues to track them moving forward?
Many thanks @AakashGfude for all of your help on this one
Thanks @choldgraf, I will update the version in a new PR once 0.13 Is released and reiterate if UX issues. Hopefully, should not be a lot. |
sphinx-book-theme 1.x apparently has an undocumented breaking change in executablebooks/sphinx-book-theme#668 -- `logo_only` is no longer supported, and instead there is a `logo` option that I haven't looked into. I'm not sure what we want, but I'd rather unbreak and move on so that we at least don't have a broken docs build.
…ays fixed (#355) The output of python-library had a number of lint, quality, and test failures. This commit 1) fixes all of those failures, and 2) ensures that we have test coverage of those quality checks so that we don't regress in the future. Fixes to quality checks and unit tests: - Don't try to pylint manage.py in base template, since not all of them have one, and this causes a quality check failure for python-library. Simply removing the explicit reference is fine, since it's already covered by `*.py`. - Add a placeholder test so pytest will succeed for python-library. (It fails if there are no tests.) This placeholder test is marked to be skipped, which will hopefully stand out a bit in the resulting repo's test runs. - Move doc theme overrides CSS to base template (deleting from layered cookiecutters). There's a line in python-template's doc config that expects a _static dir (`html_static_path = ['_static']`) but there's no dir in the base, so docs build breaks for python-library (which did not have a copy of the CSS in a _static dir). Moving the CSS to the base fixes this so all cookiecutters can use it. - Remove `logo_only` from base docs config, as it was breaking the docs build for python-library. - sphinx-book-theme 1.x apparently has an undocumented breaking change in executablebooks/sphinx-book-theme#668 -- `logo_only` is no longer supported, and instead there is a `logo` option that I haven't looked into. I'm not sure what we want, but I'd rather unbreak and move on so that we at least don't have a broken docs build. - Prevent duplicate target error in various READMEs. - The "Getting Help" section header creates an implicit duplicate target that conflicts with the `_Getting Help:` reference. Rather than doing something more complicated, just inline the link. (This required using a double-underscore link to make it anonymous, otherwise you *still* get an implicit target!) - But also this "getting started" text was copied into the cookiecutter readmes, which are *instructions* for using the cookiecutters, not used in the template output. This duplicates the root readme in the repo, so I removed that text from those readmes. (Same for in the repo's own readme.) Other improvements: - Simplify python-library's `test_upgrade` and `test_quality` unit tests to just be a single call to `make upgrade requirements test-all`. This allows us to exercise the cookiercutter's Makefile and run all of its quality checks without reproducing the full list of checks in test code. (I haven't tackled the other cookiecutters yet, since some of them actually have test failures if you try to use this approach, or don't yet have a `make test-all` or equivalent.) - Include the docs build and validation in python-library's unit tests. - Update django-app's placeholder test to match the skipped one in cookiecutter-python-library. - Remove deprecated `--recursive` arg from isort calls. (Redundant in isort 5+ and causes a warning.) Not related to python-library cc tests, but easy fix.
This adds support for much of the latest versions of our dependencies, and also removes JQuery since it is now removed in the latest version of Sphinx.
Todo
;
after copyright symbolReferences