-
Notifications
You must be signed in to change notification settings - Fork 8
Render usage.sh with sphinx-gallery #590
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This pull request streamlines the usage documentation by consolidating the primary example to a single shell script and updating the Sphinx Gallery configuration accordingly.
- Removed the separate reST file for usage and migrated its content to usage.sh.
- Updated the Sphinx Gallery configuration to include the basic_usage examples.
- Adjusted navigation links in the documentation index to point to the new usage file.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
File | Description |
---|---|
examples/basic_usage/usage.sh | Updated to include usage instructions and Sphinx Gallery cell markers; note the removal of the original shebang. |
docs/src/getting-started/usage.rst | Removed the file in favor of the consolidated usage.sh example. |
docs/src/getting-started/index.rst | Updated link reference to point to the new usage file. |
docs/generate_examples/conf.py | Modified filename/ignore patterns and added basic_usage to examples and gallery directories. |
Comments suppressed due to low confidence (1)
examples/basic_usage/usage.sh:1
- The file no longer includes a valid shebang line, which may cause issues when running it in the tox job; consider adding a proper shebang (e.g., #!/bin/bash) at the top of the file.
# .. _label_basic_usage:
27a49a6
to
57258cd
Compare
Rendered page can be checked here: https://metatrain--590.org.readthedocs.build/en/590/examples/basic_usage/usage.html old version is here https://metatensor.github.io/metatrain/latest/getting-started/usage.html |
I don't understand why is this a bash script in the first place? Are we actually testing something by running it? Otherwise I would just transform this into a normal rst file and not have the added complexity =) |
Yes we run it! I think a pure RST file is very very dangerous for us. We change the API very rapidly and most of time nobody cares about the docs. If we just have a pure rst file it will be broken within a couple of days. |
I mean, this is only smoke-testing the CLI API, right? I though we also had CLI API tests in Python elsewhere |
Yes, it is a smoke test. Lines 155 to 158 in 976a66d
But better a smoke test than no test. If Sphinx-Gallery allows us to nicely render a |
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.
Looks good, one other small nitpick on tox and this is then good to go
Should make things easier to maintain. We now only have one file and no linking between two.
sphinx-gallery
does not run the file for us. We still run it in the tox job.📚 Documentation preview 📚: https://metatrain--590.org.readthedocs.build/en/590/