-
Notifications
You must be signed in to change notification settings - Fork 617
Add test to verify tutorial table of contents #1653
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
Conversation
gabrieldemarmiesse
left a comment
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.
Thank you for the pull request! I'm wondering if it would be possible to generate automatically the docs/tutorials/_toc.yaml. Maybe in the build_docs.py? Since we already run the build_docs.py at each PR, that would ensure the code works. What do you think?
| from pathlib import Path | ||
|
|
||
|
|
||
| def get_yaml_links(): |
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.
Maybe it would be cleaner to have that as a pytest test in tools/testing/check_source_code.py? Or in a separate file in this directory? Then we run everything with pytest in one single Github actions job.
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.
Good idea, will refactor the test to be in standard testing location.
@MarkDaoust @lamberta Wanted to clarify how the docs team pulls the tutorials into the website. It looks as though We'd like to automate the generation of the tutorial TOC, but need to know where we can insert that so that docs team will pick it up. |
|
I like the idea of verifying a tutorials entry in the _toc.yaml file using GitHub Actions CI---this could be useful to many TensorFlow projects. As far as auto-generating the _toc.yaml file, my concern is the title. The page title can be longer and a bit more descriptive (for users and SEO), but leftnav width is limited and nav entries look bad when they spill over multiple lines. You could store a different nav-title somewhere, but that seems unnecessary if you're already verifying the TOC is up-to-date in the CI. @MarkDaoust can confirm, but I believe build_docs.py is run when importing the "API rule",, which is usually always run at the same times as the "narrative rule", but doesn't have to be. I do not know if we provide a hook when the narrative docs are imported, but maybe that's an option. |
Exactly, it's two separate tasks: "build the api-reference", and "import the docs directory", so there isn't really any hook you can use here. Automating it would have to be done on our side. Obviously it's not too complicated if we have a good way to get a title and choose an order Subsites often forget to update their TOC. So we either need more testing or an auto-toc. |
|
While we wait for an implementation in the tensorflow tooling, I think the best way forward for us to to have a light check in tools/testing/check_source_code.py |
No description provided.