-
Notifications
You must be signed in to change notification settings - Fork 351
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
Updated markdown documentation files #2037
Updated markdown documentation files #2037
Conversation
Changes: - Replaced relative image sources with absolute image URLs so that images load when viewed at https://pypi.org/project/MaterialX/ - Streamlined headline levels: page title (if any) uses `#`, first level uses `##`, second level uses `###`, etc. - Used markdown tables with table headers for formatting image galleries - Streamlined figure captions to appear _below_ figures - Used HTML comment syntax for comments in XML blocks - Other minor formatting changes, e.g. formatting the `MaterialXView` executable in a code font Split from AcademySoftwareFoundation#1567. Update AcademySoftwareFoundation#342. Signed-off-by: Stefan Habel <[email protected]>
…Ls as well. Signed-off-by: Stefan Habel <[email protected]>
ae2b4ab
to
d2e5281
Compare
This PR adds support for generating Python API documentation in HTML format using Sphinx from the MaterialX Python that are built in the `lib/` directory. A new CMake build option named `MATERIALX_BUILD_PYTHON_DOCS` allows developers to turn generating Python API documentation on. When `MATERIALX_BUILD_PYTHON_DOCS` is set to `ON`, `MATERIALX_BUILD_PYTHON` is set to `ON` as well, ensuring we have Python modules for which to build the Python API docs. The core functionality of generating Python API documentation lives in a new directory named `documents/PythonAPI/`. It is controlled with a new `CMakeLists.txt` file in that directory, which defines a new target named `MaterialXDocsPython`, similar to the existing target `MaterialXDocs` that generates API documentation for the MaterialX C++ API. To facilitate the curation and addition of docstrings in the implementation files within `source/PyMaterialX/`, this PR adds a new helper macro named `PYMATERIALX_DOCSTRING` that allows developers of Python modules to define docstrings using the following pattern: ```cpp PYMATERIALX_DOCSTRING(R"docstring( ...markdown text here... )docstring"); ``` Revised docstrings for modules and classes are to be added in subsequent PRs separately. Documentation in markdown format from the existing `DeveloperGuide` is integrated into the new Python API documentation by way of symlinking the four main `.md` files into the `documents/PythonAPI/sources/` directory from which Sphinx generates the resulting HTML documentation. Warnings that are issued when generating the documentation via Sphinx are to be addressed in a separate PR for the markdown files: AcademySoftwareFoundation#2037 To build the docs from scratch on macOS, I've used the following build script, naming it `build.sh` in the `MaterialX` checkout directory: ```bash #!/usr/bin/env tcsh echo build.sh: Updating Git submodules... git submodule update --init --recursive # Create and activate a virtual environment for installing Python dependencies python3 -m venv /tmp/venv source /tmp/venv/bin/activate.csh echo build.sh: Installing dependencies... python3 -m pip install myst_parser # https://pypi.org/project/myst-parser/ echo build.sh: Making build directory and changing into it... mkdir build cd build echo build.sh: Configuring... cmake .. \ --fresh \ -DCMAKE_OSX_SYSROOT=/Library/Developer/CommandLineTools/SDKs/MacOSX15.0.sdk \ -DMATERIALX_BUILD_PYTHON=ON \ -DMATERIALX_BUILD_VIEWER=ON \ -DMATERIALX_BUILD_GRAPH_EDITOR=ON \ -DMATERIALX_BUILD_DOCS=ON \ -DMATERIALX_BUILD_PYTHON_DOCS=ON \ -DMATERIALX_BUILD_TESTS=ON \ && \ echo build.sh: Building... \ && \ cmake --build . -j 8 \ && \ echo build.sh: Building target MaterialXDocs... \ && \ cmake --build . --target MaterialXDocs \ && \ echo build.sh: Building target MaterialXDocsPython... \ && \ cmake --build . --target MaterialXDocsPython \ && \ afplay /System/Library/Sounds/Blow.aiff # Deactivate the virtual environment deactivate ``` The build output currently ends with the following messages: ```python The parsed MaterialX Python API consists of: * 11 modules * 48 functions * 139 classes * 1175 methods * 6 exception types WARNING: 48 functions look like they do not have docstrings yet. WARNING: 1019 methods look like they do not have docstrings yet. WARNING: 32 functions look like their parameters have not all been named using `py::arg()`. WARNING: 499 methods look like their parameters have not all been named using `py::arg()`. build succeeded, 168 warnings. The HTML pages are in .. [100%] Built target MaterialXDocsPython ``` Split from AcademySoftwareFoundation#1567. Update AcademySoftwareFoundation#342. Signed-off-by: Stefan Habel <[email protected]>
Thanks for this proposal, @StefanHabel! I think I'll need to spend some time with these new visual ideas to form a strong impression, though my initial sense is that the captions over the image rows on the front page (e.g. Would it work visually if we adopted your new tabular organization of the image rows, but maintained the original placement of the |
...with small captions below images. Also streamlining figure text formatting to format all colons in bold. Signed-off-by: Stefan Habel <[email protected]>
...but leaving those in `ShaderGeneration.md` as-is. Signed-off-by: Stefan Habel <[email protected]>
Thanks for the feedback @jstone-lucasfilm! The different markdown pages currently use two different placements for the figure text:
I've added two more commits to use HTML tables for these image galleries, with two different placements of figure texts -- above and below their corresponding figures -- so that we can compare the appearance. In both cases, I've moved the descriptions of each image to small text below the respective image. Above: Below: Personally, I'm more used to text describing a figure appearing below the figure. The current state in this branch is for the figure texts to appear above the figures, as before, but leaving those in |
Maybe we could add a colon at the end of figure texts that appear above figures, to make it clearer that they pertain to the respective figure below? e.g.
-**Figure 1:** Procedural and uniform materials in the MaterialX viewer
+**Figure 1:** Procedural and uniform materials in the MaterialX viewer: |
@StefanHabel Could it be that the individual labels for images (e.g. Just as a reference point, here's the current layout of the front page, which doesn't make use of these individual labels: |
Signed-off-by: Stefan Habel <[email protected]>
Signed-off-by: Stefan Habel <[email protected]>
Signed-off-by: Stefan Habel <[email protected]>
The latest version of the front page looks good to me, thanks @StefanHabel! I wonder if we can reduce the buffer widths in the image rows, minimizing the empty space between the images, but I don't believe that should be a blocking issue. I'll start taking a closer look at the details of this change, and we can iterate from there. |
Signed-off-by: Jonathan Stone <[email protected]>
Copying tables from dd567d8: AcademySoftwareFoundation@dd567d8 Signed-off-by: Stefan Habel <[email protected]>
It's those large white blocks around each image that I was hoping we could remove, if there's an option in the HTML to allow this. |
Understood! We should be able to remove those by using We can also remove the table border using I'll experiment locally... |
Much appreciated, @StefanHabel, and overall this looks like a great set of improvements! I particularly enjoyed the update of a specification link that was still pointing to our v1.36 PDF instead of the GitHub Markdown for MaterialX v1.39. :D |
Signed-off-by: Stefan Habel <[email protected]>
Thanks Jonathan! Regarding linking to the MaterialX Specification, we currently use the following two different links (as of the current state of this PR):
Should both links point to the same? I've added ba35d10 to tweak the image gallery spacing. This is what the Viewer page now looks like in the context of the Python API docs: |
@StefanHabel That looks perfect in the screenshot above, though I get different results when I view the front page README of your fork of MaterialX: https://github.com/StefanHabel/MaterialX/tree/%23342-markdown-changes Let me know what you see on your side, and we can figure out what might account for that difference. For linking to the specification, I think your choice to use https://materialx.org/Specification.html is the right one, as that covers the full set of specification documents as well as its history. |
…dths. Signed-off-by: Stefan Habel <[email protected]>
…ification link in `MainPage.md` as well. Signed-off-by: Stefan Habel <[email protected]>
We're actually seeing this uneven spacing (possibly due to rounding errors) in the current version of |
Ah, that latest version looks quite good! I think this achieves the goal of maintaining the appearance of the original image rows, using the more adaptable table-based implementation that you've proposed. The slight variation in spacing doesn't seem problematic, and I think you're right that it was originally present in some viewing environments as well. |
Oh, I've just caught up with the posts above, and I see that it's no longer "table based", but I think that your latest proposed change is a good compromise in any case. |
Signed-off-by: Jonathan Stone <[email protected]>
Signed-off-by: Jonathan Stone <[email protected]>
Signed-off-by: Jonathan Stone <[email protected]>
Signed-off-by: Jonathan Stone <[email protected]>
Signed-off-by: Jonathan Stone <[email protected]>
Signed-off-by: Jonathan Stone <[email protected]>
Signed-off-by: Jonathan Stone <[email protected]>
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.
This change looks good to me, thanks @StefanHabel!
923ffd9
into
AcademySoftwareFoundation:main
Thanks for collaborating on this and merging it @jstone-lucasfilm! 🙌 Another positive side effect of the use of relative widths for the image galleries is that on mobile (and generally screens of varying sizes) the pages look much closer to the desktop experience:
(Also noting the change of headline level.) |
This PR adds support for generating Python API documentation in HTML format using Sphinx from the MaterialX Python that are built in the `lib/` directory. A new CMake build option named `MATERIALX_BUILD_PYTHON_DOCS` allows developers to turn generating Python API documentation on. When `MATERIALX_BUILD_PYTHON_DOCS` is set to `ON`, `MATERIALX_BUILD_PYTHON` is set to `ON` as well, ensuring we have Python modules for which to build the Python API docs. The core functionality of generating Python API documentation lives in a new directory named `documents/PythonAPI/`. It is controlled with a new `CMakeLists.txt` file in that directory, which defines a new target named `MaterialXDocsPython`, similar to the existing target `MaterialXDocs` that generates API documentation for the MaterialX C++ API. To facilitate the curation and addition of docstrings in the implementation files within `source/PyMaterialX/`, this PR adds a new helper macro named `PYMATERIALX_DOCSTRING` that allows developers of Python modules to define docstrings using the following pattern: ```cpp PYMATERIALX_DOCSTRING(R"docstring( ...markdown text here... )docstring"); ``` Revised docstrings for modules and classes are to be added in subsequent PRs separately. Documentation in markdown format from the existing `DeveloperGuide` is integrated into the new Python API documentation by way of symlinking the four main `.md` files into the `documents/PythonAPI/sources/` directory from which Sphinx generates the resulting HTML documentation. Warnings that are issued when generating the documentation via Sphinx are to be addressed in a separate PR for the markdown files: AcademySoftwareFoundation#2037 To build the docs from scratch on macOS, I've used the following build script, naming it `build.sh` in the `MaterialX` checkout directory: ```bash echo build.sh: Updating Git submodules... git submodule update --init --recursive python3 -m venv /tmp/venv source /tmp/venv/bin/activate.csh echo build.sh: Installing dependencies... python3 -m pip install myst_parser # https://pypi.org/project/myst-parser/ echo build.sh: Making build directory and changing into it... mkdir build cd build echo build.sh: Configuring... cmake .. \ --fresh \ -DCMAKE_OSX_SYSROOT=/Library/Developer/CommandLineTools/SDKs/MacOSX15.0.sdk \ -DMATERIALX_BUILD_PYTHON=ON \ -DMATERIALX_BUILD_VIEWER=ON \ -DMATERIALX_BUILD_GRAPH_EDITOR=ON \ -DMATERIALX_BUILD_DOCS=ON \ -DMATERIALX_BUILD_PYTHON_DOCS=ON \ -DMATERIALX_BUILD_TESTS=ON \ && \ echo build.sh: Building... \ && \ cmake --build . -j 8 \ && \ echo build.sh: Building target MaterialXDocs... \ && \ cmake --build . --target MaterialXDocs \ && \ echo build.sh: Building target MaterialXDocsPython... \ && \ cmake --build . --target MaterialXDocsPython \ && \ afplay /System/Library/Sounds/Blow.aiff deactivate ``` The build output currently ends with the following messages: ```python The parsed MaterialX Python API consists of: * 11 modules * 48 functions * 139 classes * 1175 methods * 6 exception types WARNING: 48 functions look like they do not have docstrings yet. WARNING: 1019 methods look like they do not have docstrings yet. WARNING: 32 functions look like their parameters have not all been named using `py::arg()`. WARNING: 499 methods look like their parameters have not all been named using `py::arg()`. build succeeded, 168 warnings. The HTML pages are in .. [100%] Built target MaterialXDocsPython ``` Split from AcademySoftwareFoundation#1567. Update AcademySoftwareFoundation#342. Signed-off-by: Stefan Habel <[email protected]>
For the record/archive: ✅ This PR has resolved the following warnings that were previously issued by Sphinx/MyST-Parser when integrating the markdown pages into the Python API documentation (see #2038): GraphEditor.md.rst:5: WARNING: Non-consecutive header level increase; H1 to H3 [myst.header]
MainPage.md.rst:5: WARNING: Non-consecutive header level increase; H1 to H3 [myst.header]
MainPage.md.rst:12: WARNING: Non-consecutive header level increase; H1 to H3 [myst.header]
MainPage.md.rst:22: WARNING: Non-consecutive header level increase; H1 to H3 [myst.header]
MainPage.md.rst:57: WARNING: Non-consecutive header level increase; H1 to H3 [myst.header]
MainPage.md.rst:61: WARNING: Non-consecutive header level increase; H1 to H3 [myst.header]
MainPage.md.rst:82: WARNING: Non-consecutive header level increase; H1 to H3 [myst.header]
ShaderGeneration.md:8: WARNING: image file not readable: documents/Images/shadergen.png [image.not_readable]
Viewer.md.rst:5: WARNING: Non-consecutive header level increase; H1 to H3 [myst.header]
GraphEditor.md:23: WARNING: 'myst' cross-reference target not found: '../../resources/Materials/Examples' [myst.xref_missing]
ShaderGeneration.md:4: WARNING: 'myst' cross-reference target not found: '/source/MaterialXGenGlsl' [myst.xref_missing]
ShaderGeneration.md:4: WARNING: 'myst' cross-reference target not found: '/source/MaterialXGenOsl' [myst.xref_missing]
ShaderGeneration.md:82: WARNING: 'myst' cross-reference target not found: '../Specification/MaterialX.v1.36.Spec.pdf' [myst.xref_missing]
ShaderGeneration.md:34: WARNING: Lexing literal_block '// Nodedef elements for node <add>\n<nodedef name="ND_add_float" node="add">\n <input name="in1" type="float" />\n <input name="in2" type="float" />\n <output name="out" type="float" defaultinput="in1" />\n</nodedef>\n<nodedef name="ND_add_color3" node="add" type="color3">\n <input name="in1" type="color3" />\n <input name="in2" type="color3" />\n <output name="out" type="color3" defaultinput="in1" />\n</nodedef>\n<... more types ...>\n\n// Implementation elements for node <add>\n<implementation name="IM_add_float" nodedef="ND_add_float" file="mx_add.inline" />\n<implementation name="IM_add_color3" nodedef="ND_add_color3" file="mx_add.inline" />\n<... more types ...>\n\n// Nodedef elements for node <mix>\n<nodedef name="ND_mix_float" node="mix">\n <input name="fg" type="float" />\n <input name="bg" type="float" />\n <input name="mix" type="float" />\n <output name="out" type="float" defaultinput="bg" />\n</nodedef>\n<nodedef name="ND_mix_color3" node="mix">\n <input name="fg" type="color3" />\n <input name="bg" type="color3" />\n <input name="mix" type="color3" />\n <output name="out" type="color3" defaultinput="bg" />\n</nodedef>\n<... more types ...>\n\n// Implementation elements for node <mix>\n<implementation name="IM_mix_float" nodedef="ND_mix_float" sourcecode="mix({{bg}}, {{fg}}, {{mix}})" />\n<implementation name="IM_mix_color3" nodedef="ND_mix_color3" sourcecode="mix({{bg}}, {{fg}}, {{mix}})" />\n<... more types ...>\n' as "xml" resulted in an error at token: 'm'. Retrying in relaxed mode. [misc.highlighting_failure]
Viewer.md:34: WARNING: 'myst' cross-reference target not found: '../../resources/Geometry' [myst.xref_missing]
Viewer.md:40: WARNING: 'myst' cross-reference target not found: '../../resources/Materials/Examples/StandardSurface' [myst.xref_missing]
Viewer.md:40: WARNING: 'myst' cross-reference target not found: '../../resources/Materials/Examples/UsdPreviewSurface' [myst.xref_missing]
Viewer.md:46: WARNING: 'myst' cross-reference target not found: '../../resources/Materials/Examples/StandardSurface/standard_surface_look_brass_tiled.mtlx' [myst.xref_missing]
Viewer.md:50: WARNING: 'myst' cross-reference target not found: '../../resources/Lights' [myst.xref_missing] |
This PR adds support for generating Python API documentation in HTML format using Sphinx from the MaterialX Python that are built in the `lib/` directory. A new CMake build option named `MATERIALX_BUILD_PYTHON_DOCS` allows developers to turn generating Python API documentation on. When `MATERIALX_BUILD_PYTHON_DOCS` is set to `ON`, `MATERIALX_BUILD_PYTHON` is set to `ON` as well, ensuring we have Python modules for which to build the Python API docs. The core functionality of generating Python API documentation lives in a new directory named `documents/PythonAPI/`. It is controlled with a new `CMakeLists.txt` file in that directory, which defines a new target named `MaterialXDocsPython`, similar to the existing target `MaterialXDocs` that generates API documentation for the MaterialX C++ API. To facilitate the curation and addition of docstrings in the implementation files within `source/PyMaterialX/`, this PR adds a new helper macro named `PYMATERIALX_DOCSTRING` that allows developers of Python modules to define docstrings using the following pattern: ```cpp PYMATERIALX_DOCSTRING(R"docstring( ...markdown text here... )docstring"); ``` Revised docstrings for modules and classes are to be added in subsequent PRs separately. Documentation in markdown format from the existing `DeveloperGuide` is integrated into the new Python API documentation by way of symlinking the four main `.md` files into the `documents/PythonAPI/sources/` directory from which Sphinx generates the resulting HTML documentation. Warnings that are issued when generating the documentation via Sphinx are to be addressed in a separate PR for the markdown files: AcademySoftwareFoundation#2037 To build the docs from scratch on macOS, I've used the following build script, naming it `build.sh` in the `MaterialX` checkout directory: ```bash echo build.sh: Updating Git submodules... git submodule update --init --recursive python3 -m venv /tmp/venv source /tmp/venv/bin/activate.csh echo build.sh: Installing dependencies... python3 -m pip install myst_parser # https://pypi.org/project/myst-parser/ echo build.sh: Making build directory and changing into it... mkdir build cd build echo build.sh: Configuring... cmake .. \ --fresh \ -DCMAKE_OSX_SYSROOT=/Library/Developer/CommandLineTools/SDKs/MacOSX15.0.sdk \ -DMATERIALX_BUILD_PYTHON=ON \ -DMATERIALX_BUILD_VIEWER=ON \ -DMATERIALX_BUILD_GRAPH_EDITOR=ON \ -DMATERIALX_BUILD_DOCS=ON \ -DMATERIALX_BUILD_PYTHON_DOCS=ON \ -DMATERIALX_BUILD_TESTS=ON \ && \ echo build.sh: Building... \ && \ cmake --build . -j 8 \ && \ echo build.sh: Building target MaterialXDocs... \ && \ cmake --build . --target MaterialXDocs \ && \ echo build.sh: Building target MaterialXDocsPython... \ && \ cmake --build . --target MaterialXDocsPython \ && \ afplay /System/Library/Sounds/Blow.aiff deactivate ``` The build output currently ends with the following messages: ```python The parsed MaterialX Python API consists of: * 11 modules * 48 functions * 139 classes * 1175 methods * 6 exception types WARNING: 48 functions look like they do not have docstrings yet. WARNING: 1019 methods look like they do not have docstrings yet. WARNING: 32 functions look like their parameters have not all been named using `py::arg()`. WARNING: 499 methods look like their parameters have not all been named using `py::arg()`. build succeeded, 168 warnings. The HTML pages are in .. [100%] Built target MaterialXDocsPython ``` Split from AcademySoftwareFoundation#1567. Update AcademySoftwareFoundation#342. Signed-off-by: Stefan Habel <[email protected]>
Changes:
#
, first level uses##
, second level uses###
, etc.MaterialXView
executable in a code fontSplit from #1567.
Update #342.