-
-
Notifications
You must be signed in to change notification settings - Fork 488
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
DOC, DEP: Remove custom :button_text
directive option, bump jupyterlite-sphinx
#737
DOC, DEP: Remove custom :button_text
directive option, bump jupyterlite-sphinx
#737
Conversation
:button_text
directive option, bump jupyterlite-sphinx
:button_text
directive option, bump jupyterlite-sphinx
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.
Thanks @agriyakhetarpal! This LGTM and seems to render fine on the RTD preview, so let's give this a go.
The Sphinx pin: pretty sure it was because of something that broke in some recent version (6.x had a lot of changes). But if 7.3 works fine, sure let's upgrade to that. Perhaps let's make it the minimum requirement then, to skip 6.x. Better in a separate PR I think, because it's a separate issue. Could you open a new PR for it?
@@ -90,7 +90,6 @@ computations can be performed with the `periodization`_ mode: | |||
**Example:** | |||
|
|||
.. try_examples:: | |||
:button_text: Try it in your browser! |
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.
@agriyakhetarpal on this page the buttons disappeared. Maybe something went wrong here, could you check please?
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.
That's quite strange. I'm able to see the two buttons on https://pywavelets.readthedocs.io/en/latest/ref/signal-extension-modes.html, which is mapped to this file, and I verified that this file locally also contains two buttons. Could you please try with a fresh browser instance, or reload this page with caches disabled (Ctrl+Shift+R), perhaps?
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.
Sorry, wrong page. I was showing this to someone else during a meeting, and we were looking at Usage Examples. I thought we had buttons there? It has a modes
pages too: https://pywavelets.readthedocs.io/en/latest/regression/modes.html
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.
Ah, the pages under the "Usage Examples" section need work to get them working at this time; just the API reference has been implemented. We shall need something like MyST-NB and then convert everything to notebooks in order for them to run with the NotebookLite
directive. I reckon that Jupytext notebooks will be better than converting to IPyNB. I can get started on this soon if you would want me to do so? We had previously left these for a later time, now sounds like a good time to pick it up.
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.
Ah, I misremembered then - that's what I get for doing things during a meeting on a Friday night.
No need to do it now if MyST-NB isn't ready yet (I'm aware of the work in SciPy on it), but once it's unblocked then yes it would be good to do it.
I reckon that Jupytext notebooks will be better than converting to IPyNB.
Oh yes, definitely some .md
format and no .ipynb
.
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.
Ah, I misremembered then - that's what I get for doing things during a meeting on a Friday night.
No worries! 😄
No need to do it now if MyST-NB isn't ready yet (I'm aware of the work in SciPy on it), but once it's unblocked then yes it would be good to do it.
Oh yes, definitely some .md format and no .ipynb.
It's ready, actually, it is just SciPy that hits some edge cases because of its custom MathJax, and we managed to find a suitable solution quite recently. The rest of the work there can now be resumed, so nothing remains blocked. I think I will consider that, plus the relatively smaller size of PyWavelets, as my sign and start the work, after which all pages under the PyWavelets documentation will be embellished with interactivity.
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.
Sounds great, thanks Agriya.
Description
This is a small PR because
jupyterlite-sphinx
has a new release (jupyterlite/jupyterlite-sphinx#171) which simplifies some things here.Addresses some issues found in #728 and is a bigger part of #706. I had committed this last month, now rebased on top of the latest changes.
Changes made
jupyterlite-sphinx
version to 0.14.0 corresponding to the new release, and:button_text:
option from the.. try_examples::
directives everywhere in the source code, since the directive can now use the global configuration inconf.py
.Additional context
While the documentation is building alright right now, the upper pin on
docutils
restricts us to Sphinx v5.3.0, while the seemingly more stable v7.3.7 is available. Should we remove this pin? I removed it and built the docs again without issues, so, it doesn't seem to be needed. @rgommers, do we still need this pin or can we relax it? I can't find anything with a quick search suggestive of why this pin exists; if it makes sense, I am happy to do this in the same PR.