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

DOC: Migrate to pydata-sphinx-theme #704

Merged

Conversation

agriyakhetarpal
Copy link
Collaborator

Description

This PR prepares the transition of the PyWavelets documentation towards interactivity through in-line usage examples, which will be added in follow-up PRs through the integration of JupyterLite notebooks.

Several changes have been introduced, including but not limited to:

  1. Replacement of the "nature" theme by the PyData Sphinx Theme, prevalent amongst Scientific Python packages
  2. Two useful utility-based extensions, sphinx-togglebutton and sphinx-copybutton were added
  3. The HTML theme options based on the available ones in the theme were added to customise various sections of the webpages (sidebars, navbar, footer, etc.)
  4. Removal of older templates used to customise the sidebars and their icons
  5. a few minor improvements to the directives present in the documentation

@agriyakhetarpal
Copy link
Collaborator Author

Since we do not have RTD checks for now, I am happy to add another entry to the PR workflows where we can run the doctests, build the doc, and check if they are warning-free – if this would be needed

@agriyakhetarpal
Copy link
Collaborator Author

For our interactive documentation goal, should I open an issue and add a task list there about the other deliverables, which shall be accommodated in further PRs?

@rgommers
Copy link
Member

rgommers commented Mar 4, 2024

Since we do not have RTD checks for now, I am happy to add another entry to the PR workflows where we can run the doctests, build the doc, and check if they are warning-free – if this would be needed

That sounds nice, thank you. It would avoid merging things that then not build on RTD later.

@rgommers
Copy link
Member

rgommers commented Mar 4, 2024

For our interactive documentation goal, should I open an issue and add a task list there about the other deliverables, which shall be accommodated in further PRs?

Sure, a single tracking issue would be helpful.

@agriyakhetarpal
Copy link
Collaborator Author

agriyakhetarpal commented Mar 5, 2024

Thank you for your input – all tests are passing except for the AppVeyor build which is pending, this should be ready for a review now.

@rgommers rgommers added this to the v1.6.0 milestone Mar 8, 2024
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Thanks @agriyakhetarpal. This looks quite good. I did some local testing - only a few comments.

The doc build shows one warning, could you check this?

The default value for `navigation_with_keys` will change to `False` in the next release. If you wish to preserve the old behavior for your site, set `navigation_with_keys=True` in the `html_theme_options` dict in your `conf.py` file. Be aware that `navigation_with_keys = True` has negative accessibility implications: https://github.com/pydata/pydata-sphinx-theme/issues/1492

The pages all look good except for the "Signal extension modes" one which has a rendering issue:

image

Could you please take that along while we are at it improving the docs here?

doc/source/conf.py Show resolved Hide resolved
@rgommers
Copy link
Member

rgommers commented Mar 8, 2024

The merge of gh-687 also caused a few merge conflicts - could you please fix those too @agriyakhetarpal?

@agriyakhetarpal
Copy link
Collaborator Author

The doc build shows one warning, could you check this?

For some reason I didn't see this one locally; we can keep navigation_with_keys as False, since that is what I assume the documentation for other libraries will be doing (or adapting to). I addressed it in 8ecb30b.

The pages all look good except for the "Signal extension modes" one which has a rendering issue:

This page in specific had an indentation issue. I addressed it in 1a38359.


P.S. I merged the master branch instead of rebasing because I didn't want to blemish your review. I am happy to rewrite the commit history or get this squash-merged – it is up to you :)

@agriyakhetarpal
Copy link
Collaborator Author

I fixed up all the tests (not sure if the doctests are actually supposed to print nothing or that is a bug?)

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Thanks for the quick turnaround. All looks good to me now, in it goes!

P.S. I merged the master branch instead of rebasing because I didn't want to blemish your review. I am happy to rewrite the commit history or get this squash-merged – it is up to you :)

Good call to do that on larger PRs, makes re-reviewing easier.

I fixed up all the tests (not sure if the doctests are actually supposed to print nothing or that is a bug?)

Your fix is correct. No idea why that test wasn't failing before - doctest isn't too reliable.

@rgommers rgommers merged commit c998ce3 into PyWavelets:master Mar 8, 2024
15 checks passed
@agriyakhetarpal agriyakhetarpal deleted the migrate-to-pydata-sphinx-theme branch March 8, 2024 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants