Add a new style of generated documentation #508

joshessman-llnl · 2021-03-29T16:27:39Z

Summary

This PR is a feature
It does the following:
- Adds a new style of Sphinx documentation generation

Depends on #450 and should not be merged until that PR is merged.

See https://axom.readthedocs.io/en/feature-essman-nested_docs/axom/inlet/docs/sphinx/nested_structs_nested_documentation.html for an example, compared to the current style: https://axom.readthedocs.io/en/feature-essman-nested_docs/axom/inlet/docs/sphinx/nested_structs_table_documentation.html

This PR also fixes #500 by adding a Writer documentation page: https://axom.readthedocs.io/en/feature-essman-nested_docs/axom/inlet/docs/sphinx/writers.html

Implement hashing or something similar to avoid collisions with hyperlinks

…ink trivial subcontainers

white238 · 2021-04-26T20:36:31Z

src/axom/inlet/SphinxWriter.cpp

+  if(isCollectionGroup(container.name()) &&
+     !sidreGroup->hasView(detail::STRUCT_COLLECTION_FLAG))
+  {
+    return;


Does the user end up knowing this is a collection of primitives from the outputted documentation?

It's not extremely clear (see https://axom.readthedocs.io/en/feature-essman-nested_docs/axom/inlet/docs/sphinx/mfem_coefficient_nested_documentation.html#attrs) but I can clean this up in a follow-up once we decide what that should look like (see also #482 (review))

white238 · 2021-04-26T20:48:30Z

src/axom/inlet/SphinxWriter.cpp

+      functions.end(),
+      [this, containerCount](const std::vector<std::string>& functionData) {
+        // Insert a hyperlink target for the name
+        m_oss << fmt::format(".. _{0}{1}:\n\n", functionData[0], containerCount);


src/axom/inlet/SphinxWriter.hpp

white238 · 2021-04-26T20:54:39Z

src/axom/inlet/SphinxWriter.hpp

+  enum class Style
+  {
+    // FIXME: These names are not clear at all
+    Table,  // Tables for the child Fields/Functions of each Container


I think nested is clear (at least how I see it) maybe focus on figuring out a better name for the overloaded "Table". Singular or Individuals? or possibly just non-nested.

Do you see anything that would be fall under nested/non-nested table? Are we jumping to complication before we need to. Would a simple boolean work for now?

I think it's within the realm of possibility that we'd want a third style of Sphinx documentation - the other consideration is readability in the constructor call:

SphinxWriter("docs.rst", "My Docs", true); // vs... SphinxWriter("docs.rst", "My Docs", SphinxWriter::Style::Nested);

src/axom/inlet/SphinxWriter.hpp

white238 · 2021-04-27T19:42:48Z

src/axom/inlet/docs/sphinx/mfem_coefficient_nested_documentation.rst

+
+
+
+List of boundary conditions


This is doubled for some reason.

Good catch, ended up just being a copy-paste error!

Duplication is still here.

agcapps

Looks good, from what I can see. Consider renaming the Style names, but that's only a style recommendation.

src/axom/inlet/SphinxWriter.hpp

kennyweiss · 2021-05-03T22:58:32Z

Quick question: Where do the indices come from in the "Table Mode" ?
E.g. "Table 2 Fields" and "Table 3 Fields" in the following:

Externally, it seems like an implementation detail that the user wouldn't understand or be able to reference in their input decks.
Or am I missing something?

EDIT: Or is this a sphinx-generated index, unrelated to inlet ? Since they're not referenced, perhaps our sphinx should disable the indices?

joshessman-llnl · 2021-05-03T23:05:17Z

Quick question: Where do the indices come from in the "Table Mode" ?
E.g. "Table 2 Fields" and "Table 3 Fields" in the following:

Externally, it seems like an implementation detail that the user wouldn't understand or be able to reference in their input decks.
Or am I missing something?

EDIT: Or is this a sphinx-generated index, unrelated to inlet ? Since they're not referenced, perhaps our sphinx should disable the indices?

Looking at the generated RST output (https://github.com/LLNL/axom/blame/4eb53eea9c7ccf85190ea8287009ac2dc5c8d164/src/axom/inlet/docs/sphinx/example1_table_documentation.rst#L34-L50), it looks like Sphinx is adding these in. I can look into disabling them (or if there exists an option for that).

white238 · 2021-05-03T23:27:50Z

src/axom/inlet/docs/sphinx/nested_structs_nested_documentation.rst

+
+
+--------------------
+Collection contents:


I'm not sure this is helpful from a user stand-point.

The idea is to express that the following section contains the schema for individual elements of the collection. Would it be more helpful to make this more verbose (e.g., "Each element of the collection can contain the following:") or to remove it entirely?

Comments need to be addressed. Don't want it merged by accident.

white238 · 2021-05-03T23:30:54Z

It's hard to tell the structure from the nested version. We should talk about ways to improve this.

white238 · 2021-05-03T23:31:29Z

src/axom/inlet/docs/sphinx/nested_structs_nested_documentation.rst

+
+**z**
+
+z-axis point to slice on


This additional section doesn't provide value over what is in the table above.

Due to Inlet's current architecture (see #508 (comment)), there isn't any additional information available for this element. One solution would be to add logic to avoid creating a separate subsection when only the description is available - does that sound reasonable?

white238 · 2021-05-03T23:32:17Z

src/axom/inlet/docs/sphinx/nested_structs_nested_documentation.rst

+translate
+---------
+
+


Is there something missing here?

Because Inlet doesn't have a separate phase for defining the schema and reading from the input file, collections (arrays/dicts) are implemented by adding schema components (scalars, etc) to each element. The docs generation logic just uses the schema from the first element, so unfortunately the documentation output will depend on what the input file looks like. In this case, the first element of this collection didn't have a "translate" member, so the information (type, etc) on this member will be incomplete.

white238 · 2021-05-03T23:36:05Z

src/axom/inlet/docs/sphinx/nested_structs_nested_documentation.rst

+Collection contents:
+--------------------
+
+Translation vector


... and here?

kennyweiss

Thanks @joshessman-llnl .

I think there are some nice ideas here, but perhaps the layout of the nested and flat tables should be improved a bit before we provide these to users?

Some specifics:

I'm finding it difficult to understand the nested structure of the tables in both the "nested" and "flat" views.
Maybe adding the subtable path as part of a table's header would help?
E.g. "operators: subtable of geometry.shapes"
The "Flat" table layout seems a bit too spread out relative to the information.
Would it be possible to have a table for each container, but for the nested containers to have links to their subtable in the RST file?
Similarly, it would be nice to have a back reference to the parent table.

kennyweiss · 2021-05-03T23:12:05Z

src/axom/inlet/docs/sphinx/writers.rst

+Using the same  ``documentation_generation.cpp`` example, the automatically generated schema can be used to assist
+with input file writing:
+
+.. image:: json_schema_example.gif


This is awesome!

kennyweiss · 2021-05-03T23:15:14Z

src/axom/inlet/docs/sphinx/nested_structs_table_documentation.rst

-=========================================
-Nested Structs Output: Input file Options
-=========================================
+==================================


Should these rst files have a comment that they were auto-generated by inlet?

kennyweiss · 2021-05-03T23:35:33Z

src/axom/inlet/docs/sphinx/nested_structs_nested_documentation.rst

+
+
+
+List of shape operations to apply


List of shape operations to apply

List of shape operations to apply

Duplicated.

kennyweiss · 2021-05-03T23:38:14Z

src/axom/inlet/SphinxWriter.cpp

+void SphinxWriter::writeNestedTables()
+{
+  // Used to avoid duplicate hyperlinks
+  int containerCount = 0;


(This references a comment I made on the Conversation page)
It looks like the table indices are inlet-generated.

My concern is that these indices are meaningful to the system (e.g. to generate unique links), but not to the user.
Is there a way to use these numbers but not display them?

It looks like Axom is enabling this explicitly:

axom/src/conf.py

Lines 91 to 93 in 858899f

# -- Option for numbering figures/tables/etc.-----------------------------------

# Note: numfig requires Sphinx (1.3+)

numfig = True

See https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-numfig for a description of the option - should we just turn this off then (the default)?

Thanks @joshessman-llnl -- I think so.

We might need to reword a few things in the mint docs, which, I think might be referencing these numbers.
(which seems a bit formal for sphinx docs to me).

I've gone ahead and tried this out - see commit 8834021

Should this maybe be moved into a separate PR?

joshessman-llnl · 2021-05-04T12:35:23Z

Maybe adding the subtable path as part of a table's header would help?
E.g. "operators: subtable of geometry.shapes"

The "Flat" table layout seems a bit too spread out relative to the information.

The "flat" layout is just the current style generated by the SphinxWriter class - after some discussion with the Serac team a while back it was determined that it might still be useful, hence why I opted to leave it as an option. Of course, that doesn't mean it can't be improved, either as part of this PR or a separate one.

Would it be possible to have a table for each container, but for the nested containers to have links to their subtable in the RST file?

If I understand correctly, this is what the "nested" style option does.

Similarly, it would be nice to have a back reference to the parent table.

I think it would definitely be possible to add the parent path/link to the parent, were you thinking that this would be useful for both the new "nested" style and the existing "flat" style?

joshessman-llnl added 7 commits March 23, 2021 09:24

feat: add nested documentation option to SphinxWriter

bbd5242

feat: added container documentation to nested Sphinx format

d5e1685

fix: ignore pindividual fields of primitive collections, don't hyperl…

05f390b

…ink trivial subcontainers

docs: add copyright to inlet docs and add writer page

94365ee

docs: reorganize docs to provide examples of both sphinx modes

2fb4c64

cleanup: add style and title selection options for the SphinxWriter

e06d1fe

Merge branch 'develop' into feature/essman/nested_docs

a38ec74

joshessman-llnl added Inlet Documentation Issues related to documentation labels Mar 29, 2021

joshessman-llnl and others added 5 commits March 29, 2021 17:06

Merge branch 'develop' into feature/essman/nested_docs

14de6f2

Merge branch 'develop' into feature/essman/nested_docs

d23082d

Merge branch 'develop' into feature/essman/nested_docs

8bae440

fix: avoid duplicated hyperlinks by using container index

e2a60fa

Merge branch 'develop' into feature/essman/nested_docs

916c980

joshessman-llnl changed the title ~~WIP: Add a new style of generated documentation~~ Add a new style of generated documentation Apr 19, 2021

white238 reviewed Apr 26, 2021

View reviewed changes

src/axom/inlet/SphinxWriter.hpp Outdated Show resolved Hide resolved

white238 reviewed Apr 26, 2021

View reviewed changes

src/axom/inlet/SphinxWriter.hpp Show resolved Hide resolved

joshessman-llnl and others added 2 commits April 27, 2021 09:25

Merge branch 'develop' into feature/essman/nested_docs

eb57947

cleanup: docs adjustments per @white238 suggestions

9fc2515

white238 previously approved these changes Apr 27, 2021

View reviewed changes

white238 reviewed Apr 27, 2021

View reviewed changes

fix: copy-paste error with docs example

7a0015d

white238 requested review from kennyweiss and agcapps April 27, 2021 20:48

agcapps approved these changes Apr 27, 2021

View reviewed changes

src/axom/inlet/SphinxWriter.hpp Show resolved Hide resolved

rename: singular -> flat

4eb53ee

white238 reviewed May 3, 2021

View reviewed changes

kennyweiss requested changes May 3, 2021

View reviewed changes

joshessman-llnl added 2 commits May 5, 2021 08:45

cleanup: table -> nested in filenames

844bb2b

docs: disable sphinx numfig option, refactor mint docs accordingly

8834021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a new style of generated documentation #508

Add a new style of generated documentation #508

joshessman-llnl commented Mar 29, 2021 •

edited

Loading

white238 Apr 26, 2021

joshessman-llnl Apr 26, 2021

white238 Apr 26, 2021

white238 Apr 26, 2021 •

edited

Loading

joshessman-llnl Apr 26, 2021

white238 Apr 27, 2021

joshessman-llnl Apr 27, 2021

white238 May 3, 2021

agcapps left a comment

kennyweiss commented May 3, 2021 •

edited

Loading

joshessman-llnl commented May 3, 2021

white238 May 3, 2021

joshessman-llnl May 4, 2021

white238 commented May 3, 2021

white238 May 3, 2021 •

edited

Loading

joshessman-llnl May 4, 2021

white238 May 3, 2021

joshessman-llnl May 4, 2021

white238 May 3, 2021

kennyweiss left a comment

kennyweiss May 3, 2021

kennyweiss May 3, 2021

kennyweiss May 3, 2021

kennyweiss May 3, 2021

joshessman-llnl May 4, 2021

kennyweiss May 4, 2021

joshessman-llnl May 5, 2021

joshessman-llnl commented May 4, 2021

	# -- Option for numbering figures/tables/etc.-----------------------------------
	# Note: numfig requires Sphinx (1.3+)
	numfig = True



		--------------------
		Collection contents:


		z

		z-axis point to slice on

		translate
		---------

Add a new style of generated documentation #508

Are you sure you want to change the base?

Add a new style of generated documentation #508

Conversation

joshessman-llnl commented Mar 29, 2021 • edited Loading

Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

white238 Apr 26, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agcapps left a comment

Choose a reason for hiding this comment

kennyweiss commented May 3, 2021 • edited Loading

joshessman-llnl commented May 3, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

white238 commented May 3, 2021

white238 May 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kennyweiss left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshessman-llnl commented May 4, 2021

joshessman-llnl commented Mar 29, 2021 •

edited

Loading

white238 Apr 26, 2021 •

edited

Loading

kennyweiss commented May 3, 2021 •

edited

Loading

white238 May 3, 2021 •

edited

Loading