Auto-Generating Tutorials Page

[isaacgsmith]

Description

I have created a script that, when we build the documentation, generates an rst file that links to all of the TARDIS tutorials. I probably have not implemented the most elegant way to run the script, so if anyone has a better way of getting it to run please let me know.

While I was changing the sidebar, I moved the API section in the sidebar. I thought it would be rather silly to make that a PR on its own, and it vaguely had to do with this PR. I made it its own commit, however, so if I need to remove that commit I will. I also added ZENODO.rst to .gitignore while I was adding tutorials.rst to .gitignore for the same reason.

Motivation and context

My goal was to create a page that lists all of the tutorials in TARDIS, which will automatically update when we add in a new tutorial (noting that all of our tutorials are under docs/io).

How has this been tested?

• Testing pipeline.
• Documentation built locally and on GitHub.

Examples

See built documentation:
https://smithis7.github.io/tardis/branch/tutorials_doc/index.html

Type of change

• Bug fix.
• New feature.
• Breaking change.
• None of the above.

Checklist

• My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • (optional) I have built the documentation on my fork following the instructions.
• I have assigned and requested two reviewers for this pull request.

[tardis-bot]

Before a pull request is accepted, it must meet the following criteria:

• Is the necessary information provided?
• Is this a duplicate PR?
  • If a new PR is clearly a duplicate, ask how this PR is different from the original PR?
  • If this PR is about to be merged, close the original PR with a link to this new PR that solved the issue.
• Does it pass existing tests and are new tests provided if required?
  • The test coverage should not decrease, and for new features should be close to 100%.
• Is the code tidy?
  • No unnecessary print lines or code comments.

[codecov]

Codecov Report

Merging #1757 (9dababa) into master (8caeca3) will decrease coverage by 3.99%.
The diff coverage is n/a.

❗ Current head 9dababa differs from pull request most recent head 2464ae1. Consider uploading reports for the commit 2464ae1 to get more accurate results

@@            Coverage Diff             @@
##           master    #1757      +/-   ##
==========================================
- Coverage   62.45%   58.45%   -4.00%     
==========================================
  Files          64       66       +2     
  Lines        5985     6687     +702     
==========================================
+ Hits         3738     3909     +171     
- Misses       2247     2778     +531     

Impacted Files	Coverage Δ
tardis/tardis/visualization/widgets/util.py	72.30% <0.00%> (-13.06%)	⬇️
tardis/tardis/util/base.py	72.19% <0.00%> (-2.16%)	⬇️
tardis/tardis/base.py	57.14% <0.00%> (ø)
tardis/tardis/montecarlo/base.py	88.77% <0.00%> (ø)
...dis/tardis/visualization/tools/convergence_plot.py	88.67% <0.00%> (ø)
tardis/tardis/io/logger/montecarlo_logger.py
tardis/tardis/grid/base.py	86.27% <0.00%> (ø)
tardis/tardis/io/logger/montecarlo_tracking.py	59.25% <0.00%> (ø)
...s/tardis/visualization/widgets/custom_abundance.py	13.83% <0.00%> (ø)
tardis/tardis/simulation/base.py	83.25% <0.00%> (+0.07%)	⬆️
... and 2 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8903c13...2464ae1. Read the comment docs.

[epassaro]

1. I don't see why this can't be implemented through conf.py, probably I'm missing something. Editing the Makefile it's not a good idea as you already pointed out.
2. Adding ZENODO.rst to .gitignore will break up the TARDIS citation. To keep this we should force push ZENODO.rst on the pipeline but I don't think that's a good idea. Probably also applies to tutorials.rst, I'm not a huge fan to adding too many things to .gitignore.

[isaacgsmith]

1. I don't see why this can't be implemented through conf.py, probably I'm missing something. Editing the Makefile it's not a good idea as you already pointed out.
2. Adding ZENODO.rst to .gitignore will break up the TARDIS citation. To keep this we should force push ZENODO.rst on the pipeline but I don't think that's a good idea. Probably also applies to tutorials.rst, I'm not a huge fan to adding too many things to .gitignore.

So to your first point, I could not figure out a way to do this through conf.py. If you have any ideas, I will try it out, but to me it seemed like the Makefile was the only way to do it.

For the second point I can revert that commit. I am purely wondering though, since ZENODO.rst is not in our repository, what would be messed up by adding it to .gitignore? I do think tutorials.rst should be ignored by git as we don't want it in the repository, just want it built during the docs build process.

[epassaro]

1. I don't see why this can't be implemented through conf.py, probably I'm missing something. Editing the Makefile it's not a good idea as you already pointed out.
2. Adding ZENODO.rst to .gitignore will break up the TARDIS citation. To keep this we should force push ZENODO.rst on the pipeline but I don't think that's a good idea. Probably also applies to tutorials.rst, I'm not a huge fan to adding too many things to .gitignore.

So to your first point, I could not figure out a way to do this through conf.py. If you have any ideas, I will try it out, but to me it seemed like the Makefile was the only way to do it.

For the second point I can revert that commit. I am purely wondering though, since ZENODO.rst is not in our repository, what would be messed up by adding it to .gitignore? I do think tutorials.rst should be ignored by git as we don't want it in the repository, just want it built during the docs build process.

Just copy and paste the script content at the end of conf.py and see what it does.

[jaladh-singhal]

This looks like a great addition - I think your code can be improved a bit (see my comments). Besides we use f-strings in most of TARDIS modules since they're more readable so please use them over strings concatenation - it'll also save you from adding \n before, after your variables (like title, description) since you can put them as f"{title}\n{description}\n"

Apart from this, I was wondering if we can reorder notebooks in list meaningfully or perhaps make the not-so-informative list of notebooks a bit more informative (& attractive 😄 ) by adding relevant visuals (for e.g. how einsteinpy does it)

[epassaro]

I don't see why it's necessary to add docs/tutorials.rst to .gitignore. If there's no alternative, please add it at the bottom of the file in a new section (we are currently using the Astropy .gitignore).

[isaacgsmith]

I don't see why it's necessary to add docs/tutorials.rst to .gitignore. If there's no alternative, please add it at the bottom of the file in a new section (we are currently using the Astropy .gitignore).

It definitely needs to be in .gitignore. We do not want that file in our repository. I do think it belongs in the sphinx section but I have placed it at the bottom as I understand the concern about not wanting to mess with the astropy stuff.

[epassaro]

I don't see why it's necessary to add docs/tutorials.rst to .gitignore. If there's no alternative, please add it at the bottom of the file in a new section (we are currently using the Astropy .gitignore).

It definitely needs to be in .gitignore. We do not want that file in our repository. I do think it belongs in the sphinx section but I have placed it at the bottom as I understand the concern about not wanting to mess with the astropy stuff.

If the file is not committed then it's not on the repository. It's just on your computer marked as "untracked". There's nothing wrong with that (it's the same situation with ZENODO.rst)

[isaacgsmith]

I don't see why it's necessary to add docs/tutorials.rst to .gitignore. If there's no alternative, please add it at the bottom of the file in a new section (we are currently using the Astropy .gitignore).

It definitely needs to be in .gitignore. We do not want that file in our repository. I do think it belongs in the sphinx section but I have placed it at the bottom as I understand the concern about not wanting to mess with the astropy stuff.

If the file is not committed then it's not on the repository. It's just on your computer marked as "untracked". There's nothing wrong with that (it's the same situation with ZENODO.rst)

Personally, I think it makes it more clear that we do not want it in the repo to have it in the .gitignore. I think we should defer to @wkerzendorf on this one.

[isaacgsmith]

@epassaro I removed any edits to .gitignore.

[jaladh-singhal]

Thanks for making changes - code looks lot neater now. Though you forgot to address my comment about using f-strings!

[jaladh-singhal]

If the file is not committed then it's not on the repository. It's just on your computer marked as "untracked". There's nothing wrong with that (it's the same situation with ZENODO.rst)

That's definitely wrong IMO @epassaro. Why? Because some new contributor built docs to add a page locally and tutorials.rst will get generated and they'll likely commit all the files together.

In general, almost auto-generated files should be gitignored. You can notice same for docs/_build, tutorials.rst is just same, difference is it got generate post-build phase.

[isaacgsmith]

If the file is not committed then it's not on the repository. It's just on your computer marked as "untracked". There's nothing wrong with that (it's the same situation with ZENODO.rst)

That's definitely wrong IMO @epassaro. Why? Because some new contributor built docs to add a page locally and tutorials.rst will get generated and they'll likely commit all the files together.

In general, almost auto-generated files should be gitignored. You can notice same for docs/_build, tutorials.rst is just same, difference is it got generate post-build phase.

That was my thinking as well.

[isaacgsmith]

If the file is not committed then it's not on the repository. It's just on your computer marked as "untracked". There's nothing wrong with that (it's the same situation with ZENODO.rst)

That's definitely wrong IMO @epassaro. Why? Because some new contributor built docs to add a page locally and tutorials.rst will get generated and they'll likely commit all the files together.

In general, almost auto-generated files should be gitignored. You can notice same for docs/_build, tutorials.rst is just same, difference is it got generate post-build phase.

I think we will discuss the .gitignore issue and if needed, address it in another PR.

[wkerzendorf]

@smithis7 @jaladh-singhal @epassaro my main criticism of the PR is that I do not think that is how files should be generated in the Sphinx API (I might be wrong). It feels very hacky to add this to the conf.py

[wkerzendorf]

https://stackoverflow.com/questions/7250659/how-to-use-python-to-programmatically-generate-part-of-sphinx-documentation

[isaacgsmith]

@wkerzendorf how I did it is the same way we generate the zenodo file.

I can use the link you suggested, but that involves (pretty messy IMO) changes to the Makefile which feels way more hacky.

[sourcery-ai]

Sourcery Code Quality Report

✅  Merging this PR will increase code quality in the affected files by 1.42%.

Quality metrics	Before	After	Change
Complexity	3.07 ⭐	3.42 ⭐	0.35 👎
Method Length	102.17 🙂	98.43 🙂	-3.74 👍
Working memory	9.46 🙂	9.34 🙂	-0.12 👍
Quality	57.62% 🙂	59.04% 🙂	1.42% 👍

Other metrics	Before	After	Change
Lines	360	374	14

Changed files	Quality Before	Quality After	Quality Change
docs/conf.py	57.62% 🙂	59.04% 🙂	1.42% 👍

Here are some functions in these files that still need a tune-up:

File	Function	Complexity	Length	Working Memory	Quality	Recommendation
docs/conf.py	create_redirect_files	3 ⭐	94 🙂	10 😞	66.84% 🙂	Extract out complex expressions

Legend and Explanation

The emojis denote the absolute quality of the code:

• ⭐ excellent
• 🙂 good
• 😞 poor
• ⛔ very poor

The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request.

---

Please see our documentation here for details on how these metrics are calculated.

We are actively working on this report - lots more documentation and extra metrics to come!

Help us improve this quality report!

[isaacgsmith]

@wkerzendorf how is this implementation? This is the more "official" way of doing it, by hooking into the sphinx build process. See https://www.sphinx-doc.org/en/master/extdev/appapi.html#events

[jaladh-singhal]

@smithis7 looks good to me except I think you must add tutotrials.rst to gitignore in this same PR.

@epassaro please read this https://www.atlassian.com/git/tutorials/saving-changes/gitignore - tutorial.rst is generated at runtime and we don't want users who have built docs locally to accidentally commit it.

• files generated at runtime, such as .log, .lock, or .tmp

[jaladh-singhal]

@smithis7 @jaladh-singhal @epassaro my main criticism of the PR is that I do not think that is how files should be generated in the Sphinx API (I might be wrong). It feels very hacky to add this to the conf.py

@wkerzendorf I don't think we have a better way for this not-so-common usecase. Also now that @smithis7 has hooked this function to build initiation event of sphinx, I can confirm this is right way to do it because I used same approach to create redirects in #1458:

tardis/docs/conf.py

Line 361 in 6a0b8db

app.connect("build-finished", create_redirect_files)