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

Move theme options to a custom sphinx theme #41

Merged
merged 1 commit into from
Mar 30, 2023

Conversation

Maetveis
Copy link
Contributor

As part of making rocm-docs-core a sphinx extension, move its theming settings to a sphinx theme rocm_docs_theme. This makes it easier to re-use. Also, the theme files no longer need to be copied to the documentation directory keeping things a little tidier.

This change is transparent to users because setup() sets the sphinx theme to this new bundled theme.

@saadrahim saadrahim requested a review from lawruble13 March 29, 2023 13:47
@lawruble13
Copy link
Collaborator

This looks excellent to me, I do have one question about the setup function in __init__.py, that should still be run correctly, yes? It includes a javascript file that is necessary for cookie compliance. Additionally I'd just like to confirm my understanding: there would now be 2 ways to use rocm-docs-core, 1) by using the current method (which would automatically set up rocm_docs.theme), or 2) by using rocm_docs.theme directly, is that accurate?

@Maetveis
Copy link
Contributor Author

This looks excellent to me, I do have one question about the setup function in __init__.py, that should still be run correctly, yes?

Yes, since this PR doesn't move all settings to the new theme only the "theming related" ones, like the custom templates css, logos etc. I have not moved the favicon setting or the one that disables the sphinx logo, as they are not strictly html_theme_options, but they could be included as visual options.

It includes a javascript file that is necessary for cookie compliance.

I think I should move that one over.

Additionally I'd just like to confirm my understanding: there would now be 2 ways to use rocm-docs-core, 1) by using the current method (which would automatically set up rocm_docs.theme), or 2) by using rocm_docs.theme directly, is that accurate?

Yes, partly. The idea would be to split the current method to several sphinx extensions, the first of which is this theme, users that only need the theme are now able to specify html_theme=rocm_docs_theme in their conf.py. Another extension could be made for running doxygen (and doxysphinx, enabling breathe etc), and probably a miscellaneous one for everything else (e.g. external-toc, notfound pages, myst) would remain the rocm_docs extension.

Backwards compatibility can be kept by making the current method enable these new extensions and setting their settings, and finally that can be dropped at a later time requiring users to instead specify the extensions they want to use.

As part of making rocm-docs-core a sphinx extension, move its theming
settings to a sphinx theme `rocm_docs_theme`. This makes it easier to
re-use. Also, the theme files no longer need to be copied to
the documentation directory keeping things a little tidier.

This change is transparent to users because `setup()` sets the
sphinx theme to this new bundled theme.
@Maetveis Maetveis mentioned this pull request Mar 30, 2023
@saadrahim saadrahim merged commit 955105a into ROCm:main Mar 30, 2023
saadrahim added a commit that referenced this pull request Mar 30, 2023
saadrahim added a commit that referenced this pull request Mar 30, 2023
@lawruble13
Copy link
Collaborator

@Maetveis This caused the following error when building on ReadTheDocs: ImportError: cannot import name '_get_theme_options' from 'pydata_sphinx_theme'

@Maetveis
Copy link
Contributor Author

Maetveis commented Mar 30, 2023

@Maetveis This caused the following error when building on ReadTheDocs: ImportError: cannot import name '_get_theme_options' from 'pydata_sphinx_theme'

Ah, this is seems like it popped up since I last built this 😓 , it looks like its this: issue pydata/pydata-sphinx-theme#1274.

I'll see if the suggested solution there works for us. It might require regenerating the lock-files (requirements.txt) however.

@Maetveis Maetveis deleted the factor_out_theme branch March 30, 2023 16:57
@Maetveis
Copy link
Contributor Author

I wonder how that even happened when supposedly we have our packages pinned in every repository.

@Maetveis Maetveis restored the factor_out_theme branch March 30, 2023 17:07
@lawruble13
Copy link
Collaborator

I wonder how that even happened when supposedly we have our packages pinned in every repository.

rocm-docs-core itself is not pinned, as it was changing quite often for a time and during early development it was convenient to not have it pinned, which may have contributed?. However we've reached the point where we should be pinning rocm-docs-core versions.

Maetveis pushed a commit to StreamHPC/rocm-docs-core that referenced this pull request Mar 30, 2023
As part of making rocm-docs-core a sphinx extension, move its theming
settings to a sphinx theme `rocm_docs_theme`. This makes it easier to
re-use. Also, the theme files no longer need to be copied to
the documentation directory keeping things a little tidier.

This change is transparent to users because `setup()` sets the
sphinx theme to this new bundled theme.

Re-land with fix and requirement for pydata-sphinx-theme 13.3
@Maetveis Maetveis deleted the factor_out_theme branch March 30, 2023 17:55
Maetveis pushed a commit to StreamHPC/rocm-docs-core that referenced this pull request Mar 31, 2023
As part of making rocm-docs-core a sphinx extension, move its theming
settings to a sphinx theme `rocm_docs_theme`. This makes it easier to
re-use. Also, the theme files no longer need to be copied to
the documentation directory keeping things a little tidier.

This change is transparent to users because `setup()` sets the
sphinx theme to this new bundled theme.

Re-land with fix and requirement for pydata-sphinx-theme 13.3
saadrahim pushed a commit that referenced this pull request Apr 11, 2023
* Move theme settings to a custom sphinx theme (#41)

As part of making rocm-docs-core a sphinx extension, move its theming
settings to a sphinx theme `rocm_docs_theme`. This makes it easier to
re-use. Also, the theme files no longer need to be copied to
the documentation directory keeping things a little tidier.

This change is transparent to users because `setup()` sets the
sphinx theme to this new bundled theme.

Re-land with fix and requirement for pydata-sphinx-theme 13.3

* Migrate setup.cfg to pyproject.toml

Having just the standard package metadata simplifies things, tools were
sometimes preferring one over the other creating confusion.

Additionally fix the theme folder not being installed

* Add readthedocs build

Add a readthedocs build mostly to verify that the package works, without
needing to create a PR in a dependent repository.

* Make theme settings overrideable

Respect any user defined settings and only provide defaults.
- Move simple defaults to theme.conf, user settings override it by default
- And prefer the users value for the more complex ones

* Move the rest of the visual settings into the theme

Moves the favicon and hiding the sphinx logo into theme, and provide
only a default value (users can now override),

* ci(lint): Add commit message linting via commitizen

The commits of pull requests are now checked with commitizen to conform to conventional commits format.

* ci(release): Add automatic version bump and pypi release

Implement automatic version bumps based on commitizen, every push to main triggers a bump if a new feature/fix is added. The new release is automatically built and published to pypi.

* docs(developer_guide): add short summary of committing rules, commitizen usage
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.

4 participants