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

Consolidate YAML API documentation in doxygen #1548

Closed
wants to merge 20 commits into from

Conversation

ischoegl
Copy link
Member

@ischoegl ischoegl commented Jul 19, 2023

Changes proposed in this pull request

This PR documents an attempt to consolidate Sphinx documentation for YAML Input within doxygen. The test emerged from discussion in Cantera/enhancements#178; the starting point was to test doxygen's capabilities for the integration of markdown files.

This PR consolidates the following information:

  • YAML Format Reference from this repository (original reST files are removed)
  • YAML Format Tutorial from Cantera/cantera-website
  • doxygen generated pages are a drop-in replacement of current content
  • updated documentation uses a doxygen flavor of markdown, which is presumably easy to port to MyST if so desired at a later point (MyST is a superset of markdown).

Other comments:

  • Sphinx reST pages are ported using pandoc and manually edited (also: some content is reshuffled to take advantage of table-of-content browsing)
  • Most cross-links are now handled by doxygen internally, where warnings are raised for broken links; it becomes trivially easy to link to C++ implementations (with corresponding docstrings)
  • The approach is obviously applicable to content that is currently located in the Cantera/cantera-website repository (e.g. C++ examples/tutorials, compiling instructions, ...)
  • Landing page is renamed to Cantera Developer API

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

Partially addresses Cantera/enhancements#178
Partial fix for Cantera/cantera-website#250 (by avoiding links between doxygen and Sphinx generated content)

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

Rendered content:

image

Format Reference

image

Works for doxygen=1.9.1 and 1.9.2. There is a known treeview bug in 19.3 until 1.9.6; this appears to be fixed in 1.9.7 although subpage tables of contents do not render for 1.9.7, see bug report (marked as fixed, but not yet released). I retrieved all doxygen versions from conda-forge.

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

@ischoegl ischoegl marked this pull request as ready for review July 19, 2023 19:23
@ischoegl ischoegl marked this pull request as draft July 20, 2023 00:27
@ischoegl ischoegl changed the title Add YAML input file API documentation to doxygen Consolidate YAML API documentation in doxygen Jul 20, 2023
@ischoegl
Copy link
Member Author

Realizing that doxylink is actually set up (:ct:, not sure what took me so long 🎉 ), I created links from Sphinx: linking to sections defined in Doxygen markdown files works nicely, see:

image

There are differences in output between Doxygen 1.9.1 and 1.9.7,
where the former uses the top-level header as the @page title
automatically. For newer Doxygen versions, those links are broken.

Also, a space in code fencing does not work in newer Doxygen.
Copy link
Member

@speth speth left a comment

Choose a reason for hiding this comment

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

I think this is interesting as a proof of concept, but I'd like to defer considering a change this significant beyond the release of Cantera 3.0 and consider it as part of the larger documentation reorganization that we were discussing in Cantera/enhancements#178. I will note a couple of concerns here, but would suggest that they don't warrant resolving immediately:

  • This seems to be based off of an out-of-date version of the website repo, and is missing some relatively recent changes I made to make clarify the components of a phase entry versus the top level elements/species/reactions sections.
  • I notice that there seems to be no syntax highlighting at all for YAML

@ischoegl
Copy link
Member Author

ischoegl commented Jul 28, 2023

Thanks for the feedback! I do agree that it is a substantial refactor, but strongly believe that merging YAML tutorial and reference makes for an improved user experience (having to look up pieces at two disjoint parts of the website is imho not ideal).

Whether consolidated YAML documentation goes into Doxygen (as proposed here) or Sphinx is probably not too consequential. For my own part, I believe Doxygen is slightly simpler, as cross references are handled internally, and tests only require running scons doxygen. Doxygen produces useful warnings about broken links by itself.

Regarding your comments:

  • There currently is no good solution for YAML syntax highlighting in Doxygen, see Support YAML syntax highlighting doxygen/doxygen#7551; it should, however, be possible using a preprocessing step. I am, however, not completely convinced that syntax highlighting adds much to YAML.
  • Regarding versions: I checked and appear to accidentally have pulled content from an unmerged fork for the MyST refactoring effort, which explains the discrepancy.

@ischoegl ischoegl added the discussion Topics that are being discussed label Jul 29, 2023
@ischoegl ischoegl marked this pull request as draft July 29, 2023 15:35
@ischoegl
Copy link
Member Author

ischoegl commented Aug 9, 2023

This is largely superseded by #1572 and #1573. Regarding other items, I opened Cantera/enhancements#182.

One bit I spent some time on (and which I believe may be worth porting) was to update the documentation of the coverage-dependent species YAML documentation, which I found utterly confusing. Edit: opened #1579.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Topics that are being discussed documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants