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

feat(doc-template): adding options for documentation themes #292

Merged
merged 21 commits into from
Jun 26, 2024

Conversation

Anavelyz
Copy link
Collaborator

@Anavelyz Anavelyz commented Jun 1, 2024

Pull Request description

Adding options for documentation themes:

  • MkDocs: Default (mkdocs), Material, Cinder, Readthedocs.
  • Sphinx: Default (Alabaster), Readthedocs, PyData
  • Jupyter-book: Default (Sphinx Book Theme), PyData-sphinx-theme, Readthedocs-sphinx-theme
  • Quarto options: Default, Cosmo, Cerulean, Materia

Fixes #108

Pull Request checklists

This PR is a:

  • bug-fix
  • new feature
  • maintenance

About this PR:

  • it includes tests.
  • the tests are executed on CI.
  • the tests generate log file(s) (path).
  • pre-commit hooks were executed locally.
  • this PR requires a project documentation update.

Author's checklist:

  • I have reviewed the changes and it contains no misspelling.
  • The code is well commented, especially in the parts that contain more
    complexity.
  • New and old tests passed locally.

Additional information

  • I managed to reproduce the problem locally from the main branch
  • I managed to test the new changes locally
  • I confirm that the issues mentioned were fixed/resolved

@@ -19,6 +19,14 @@
"command_line_interface": ["None", "Click", "Argparse"],
"documentation_engine": ["mkdocs", "sphinx", "jupyter-book", "quarto"],
"documentation_url": "{{ cookiecutter.project_url }}",
"mkdocs_theme": ["Material", "Cinder", "Mkdocs", "Readthedocs"],
Copy link
Member

Choose a reason for hiding this comment

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

I think that the first option should be "Default" for all the four variables about themes (mkdocs, sphinx and jupyter book)

Copy link
Member

@xmnlab xmnlab left a comment

Choose a reason for hiding this comment

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

for the profiles/osl.yaml

we should have by default (and visible: False) the documentation engine mkdocs, and the theme material .. osl profile will be very opinionated .. that will be the default structure that we will use for all of our projects.

@Anavelyz Anavelyz requested a review from xmnlab June 3, 2024 00:37
@Anavelyz Anavelyz force-pushed the 108-documentation-theme branch 2 times, most recently from 7803130 to ec8a5e9 Compare June 5, 2024 20:05
@xmnlab
Copy link
Member

xmnlab commented Jun 6, 2024

@Anavelyz ... thanks for working on that.
in general it looks good.

testing it with sphinx, the command makim.preview failed:


building [mo]: targets for 0 po files that are out of date
writing output... 
building [html]: targets for 7 source files that are out of date
updating environment: [new config] 7 added, 0 changed, 0 removed
/tmp/osl/osl-python-package/docs/changelog.md:2: ERROR: Document or section may not begin with a transition.
/tmp/osl/osl-python-package/docs/changelog.md:2: ERROR: Document may not end with a transition.

Notebook error:
PandocMissing in example.ipynb:
Pandoc wasn't found.
Please check that pandoc is installed:
https://pandoc.org/installing.html
Traceback (most recent call last):
  File "/tmp/tmp5u7gusuw.makim", line 1, in <module>
    sphinx-build -M html docs/ docs/_build/
Makim Error #1: 
/home/xmn/miniforge3/envs/osl-python-package/bin/xonsh 
/tmp/tmp5u7gusuw.makim

could you check that please? I didn't test the command makim docs.preview command with jupyter-book ..
for quarto ... not sure if it is something expected or not .. but for most of the themes it pops up a window with a label for render every time I click on a menu .. could you check that as well please?

thanks!

@xmnlab
Copy link
Member

xmnlab commented Jun 6, 2024

also I found some issues in the documentation, with mkdocs, as you can see in this image:

mkdocs-rtd

it is not specific for this PR, but it would be nice to fish that for this set of issues for the PSF grant.

@Anavelyz Anavelyz force-pushed the 108-documentation-theme branch 2 times, most recently from 90e0dc5 to acda7de Compare June 8, 2024 16:07
@Anavelyz
Copy link
Collaborator Author

Anavelyz commented Jun 9, 2024

@Anavelyz ... thanks for working on that. in general it looks good.

testing it with sphinx, the command makim.preview failed:


building [mo]: targets for 0 po files that are out of date
writing output... 
building [html]: targets for 7 source files that are out of date
updating environment: [new config] 7 added, 0 changed, 0 removed
/tmp/osl/osl-python-package/docs/changelog.md:2: ERROR: Document or section may not begin with a transition.
/tmp/osl/osl-python-package/docs/changelog.md:2: ERROR: Document may not end with a transition.

Notebook error:
PandocMissing in example.ipynb:
Pandoc wasn't found.
Please check that pandoc is installed:
https://pandoc.org/installing.html
Traceback (most recent call last):
  File "/tmp/tmp5u7gusuw.makim", line 1, in <module>
    sphinx-build -M html docs/ docs/_build/
Makim Error #1: 
/home/xmn/miniforge3/envs/osl-python-package/bin/xonsh 
/tmp/tmp5u7gusuw.makim

could you check that please? I didn't test the command makim docs.preview command with jupyter-book .. for quarto ... not sure if it is something expected or not .. but for most of the themes it pops up a window with a label for render every time I click on a menu .. could you check that as well please?

thanks!

I was checking and the error is because pandoc is not installed correctly, in pip there is an outdated version.

In the documentation they recommend to use conda, I tried it locally and the error disappears...

@Anavelyz Anavelyz marked this pull request as ready for review June 12, 2024 20:30
@Anavelyz Anavelyz requested review from xmnlab and removed request for xmnlab June 18, 2024 15:28
Copy link
Member

@xmnlab xmnlab left a comment

Choose a reason for hiding this comment

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

hi @Anavelyz , thanks for working on that.

in general that looks really good, thanks!

I just add two comments here.

I think that it would be nice if we standardize the value for the options, to be always without space, with dash instead, and all letters in lowercase ...

in this specific case, it would be missing just the step to convert the values to lowercase, for example, PyData -> pydata.

and of course, just in the template variable ... not in the documentation

Copy link
Member

@xmnlab xmnlab 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 working on that @Anavelyz .

I saw that a few $ was removed ... I think that probably it would be better to keep that ... but for now I will merge this PR ... so we can discuss and fix that later.

thanks! good job here! appreciate that.

@xmnlab xmnlab merged commit 428785d into osl-incubator:main Jun 26, 2024
19 checks passed
Copy link

🎉 This issue has been resolved in version 0.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

Add documentation theme for each documentation engine
2 participants