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

Render examples using Sphinx-gallery #1621

Merged
merged 22 commits into from
Oct 3, 2023
Merged

Conversation

speth
Copy link
Member

@speth speth commented Sep 23, 2023

Changes proposed in this pull request

  • Use sphinx-gallery to render the examples.
  • Use sphinx-tags to parse the example keywords and create index pages for each tag
  • Reformat example headers to use reST format so their content can be parsed by sphinx-gallery and sphinx-tags

Notes:

  • There is a lot of room to use more reST formatting in the examples. Beyond the headers, I only did a little bit of that here mostly as a test. I think doing more of that can be handled in future PRs that address a smaller number of examples and include improvements beyond just formatting updates.
  • Currently, about half the Python examples have plot output that is used to generate an "interesting" icon for the gallery. Since sphinx-gallery can't run the other language examples (at least for the time being), those galleries are full of the default image.
  • To enhance the visual appeal, I was considering finding or making some icons to use for at least some of the examples.
  • The automatic linking from the Python examples to the API docs provided by sphinx-gallery is pretty awesome.
  • This PR relies on a bleeding-edge version of sphinx-gallery that includes a pending PR to introduce the ability to parse examples in languages other than Python.
  • This PR relies on a bleeding-edge version of sphinx-tags that includes a few fixes I introduced. This extension is very small (a single file) so we could just vendor it if we're going to introduce significant customizations.
  • Running all the Python samples massively increases the time it takes to build the docs. To make local testing of updates to the API docs easier, it will probably be worth adding an option to SCons to exclude all of the example-related content.
  • For the example reformatting, I only did the new/experimental Matlab toolbox examples, rather than the "legacy" ones. I really didn't want to have to do all of those examples twice, and I'm hoping we can make the transition to the new toolbox as part of the next Cantera release.
  • Migrating the Jupyter notebook examples into this will be a separate PR.

If applicable, fill in the issue number this pull request is fixing

If applicable, provide an example illustrating new features this pull request is introducing

The page for a Python example:

Screenshot 2023-09-23 at 5 04 55 PM

Part of an index page:

Screenshot 2023-09-23 at 5 20 57 PM

Checklist

  • The pull request includes a clear description of this code change
  • Commit messages have short titles and reference relevant issues
  • Build passes (scons build & scons test) and unit tests address code coverage
  • Style & formatting of contributed code follows contributing guidelines
  • The pull request is ready for review

@codecov
Copy link

codecov bot commented Sep 23, 2023

Codecov Report

Merging #1621 (56487cd) into main (329952d) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1621      +/-   ##
==========================================
- Coverage   72.72%   72.71%   -0.01%     
==========================================
  Files         370      370              
  Lines       56287    56286       -1     
  Branches    20369    20369              
==========================================
- Hits        40932    40931       -1     
  Misses      12358    12358              
  Partials     2997     2997              
Files Coverage Δ
interfaces/cython/cantera/_cantera.pyx 100.00% <100.00%> (ø)
samples/clib/demo.c 77.77% <ø> (ø)
samples/cxx/bvp/BoundaryValueProblem.h 86.56% <ø> (ø)
samples/cxx/bvp/blasius.cpp 87.50% <ø> (ø)
samples/cxx/flamespeed/flamespeed.cpp 83.89% <ø> (ø)
samples/cxx/openmp_ignition/openmp_ignition.cpp 82.85% <ø> (ø)
samples/cxx/rankine/rankine.cpp 91.83% <ø> (ø)
samples/f77/demo_ftnlib.cpp 53.84% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@speth speth force-pushed the examples-website branch 6 times, most recently from 9ec9f2e to 795325b Compare September 23, 2023 19:10
@speth speth marked this pull request as ready for review September 23, 2023 21:38
Copy link
Member

@ischoegl ischoegl left a comment

Choose a reason for hiding this comment

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

@speth … this is terrific. While I believe that your PR over at sphinx-gallery is making good progress, I have a couple of minor nitpicks (most are lines that touch or are adjacent to pre-existing conditions).

It may make sense to wait what happens with the contribution to the upstream library?

doc/sphinx/_static/custom.css Outdated Show resolved Hide resolved
doc/sphinx/conf.py Outdated Show resolved Hide resolved
samples/clib/demo.c Show resolved Hide resolved
samples/matlab_experimental/plug_flow_reactor.m Outdated Show resolved Hide resolved
samples/matlab_experimental/plug_flow_reactor.m Outdated Show resolved Hide resolved
samples/python/onedim/diffusion_flame_batch.py Outdated Show resolved Hide resolved
samples/python/reactors/NonIdealShockTube.py Outdated Show resolved Hide resolved
samples/python/reactors/PorousMediaBurner.py Outdated Show resolved Hide resolved
samples/python/reactors/preconditioned_integration.py Outdated Show resolved Hide resolved
samples/python/surface_chemistry/sofc.py Outdated Show resolved Hide resolved
@speth
Copy link
Member Author

speth commented Sep 24, 2023

It may make sense to wait what happens with the contribution to the upstream library?

Yes, we can wait to merge this in case there are any API changes requested upstream. I don't think I want to wait for the next release of sphinx-gallery, though. I may start building another PR on top of this one with more documentation updates.

Copy link
Member

@ischoegl ischoegl left a comment

Choose a reason for hiding this comment

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

As I didn't want to pip install from GitHub repos, I downloaded the artifacts from GH Actions: things look overall great.

The only thing I noticed are mislabeled download buttons (Fortran is not Python and definitely doesn't know Jupyter 😃 ; C++/C/Matlab have the same issue ... not sure whether this needs to be fixed here or upstream):
image

I also noticed some "FortranFixed" reference that I am not sure about
image

.github/workflows/main.yml Outdated Show resolved Hide resolved
@speth
Copy link
Member Author

speth commented Sep 24, 2023

The "download all" buttons and corresponding zip files are something I still need to fix upstream.

For the individual examples, the name is the one used by Pygments (first column here), which uses "Fortran" for F90-style "free format" and "FortranFixed" for the older fixed-width format. I think it's a pretty minor wart.

@bryanwweber
Copy link
Member

Thanks for doing this Ray! I'm happy we'll be able to link to API docs from the examples, and that you've got all the examples building. Once this goes in, I'll rework the ongoing sphinx migration to use this instead of my bespoke solution.

@ischoegl
Copy link
Member

The "download all" buttons and corresponding zip files are something I still need to fix upstream.

Thanks for the upstream fix - I retriggered GH actions, and things are taken care of 🎉

For the individual examples, the name is the one used by Pygments (first column here), which uses "Fortran" for F90-style "free format" and "FortranFixed" for the older fixed-width format. I think it's a pretty minor wart.

Thanks for the explanation. It indeed is a minor wart.

@ischoegl
Copy link
Member

ischoegl commented Oct 2, 2023

🥳 ... sphinx-gallery PR is merged.

This includes just the Python "kinetics" examples to start
Sphinx-gallery creates new .rst files alongside existing manually
maintained documentation, and runs the samples in-place, generating
their output files in the source tree. Moving this all to the build
directory lets us keep the repo clean.
This was causing problems with re-importing cantera.with_units.units
while running the samples as part of the sphinx-gallery build
Copy link
Member

@ischoegl ischoegl left a comment

Choose a reason for hiding this comment

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

Thanks, @speth for this substantial addition!

@speth speth merged commit 3903cac into Cantera:main Oct 3, 2023
41 of 43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recategorize Website Examples
3 participants