-
Notifications
You must be signed in to change notification settings - Fork 408
Add pybind11 docs #2657
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
base: main
Are you sure you want to change the base?
Add pybind11 docs #2657
Conversation
| QUIET = YES | ||
| WARN_IF_UNDOCUMENTED = NO | ||
|
|
||
| GENERATE_XML = YES |
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 runs very fast and will allow anyone to parse the XML docs as desired.
| @@ -0,0 +1,586 @@ | |||
| import argparse | |||
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 can be updated / modified as desired. I would like to check it in, even if it's an internal script.
|
This looks promising, @kwokcb, and I'm linking this to our long-standing GitHub Issue for Python API documentation: For our first Dev Days event, @StefanHabel started a project to implement this feature, and he ended up making some great improvements to MaterialX Python in general, but we were never able to come up with a solution that avoided duplication between the documentation strings for C++ and Python. In your latest proposal, there's still duplication of doc strings, but it seems slightly clearer how a developer would navigate this when they add a new function to C++ and Python , as the doc strings are now part of the Python bindings themselves. I'd be very interested in thoughts from the community on this proposal, to judge whether the benefits of Python API documentation would be worth the additional burden we'd be placing on developers when they add or modify API methods. |
|
There is a variant that could be done but it's really not as developer friendly. Basically we put doc strings in separate resource files and then include that file in C++ and pybind C++ and write macros to insert. This however is a lot of work on the C++ side. The best I could come up with to build Doxygen + pybind insertion via script. It could be run as needed to update, or add in missing pybind docs before a release. |
Add a "force" replacement flag to rebuild all docs as desired.. Picked up some new docs changes since last sync.
|
I've added in an build step so doc building is done if Python is being built and Docs are building built. |
| "/source/JsMaterialX", | ||
| ] | ||
|
|
||
| wheel.exclude = [ |
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.
We don't want the html docs files for now in the package. If / when html docs are build for Python they can be included.
| [tool.scikit-build.cmake.define] | ||
| MATERIALX_BUILD_SHARED_LIBS = 'OFF' # Be explicit | ||
| MATERIALX_BUILD_PYTHON = 'ON' | ||
| MATERIALX_BUILD_DOCS = 'ON' |
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.
Auto build docs. Replaces all existing docs.
| if(MATERIALX_PYTHON_FORCE_REPLACE_DOCS) | ||
| list(APPEND PYBIND_DOCS_ARGS --force) | ||
| endif() | ||
| add_custom_target(PyBindDocs |
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.
Run doc extraction.
| "Python executable to be used in building the MaterialX Python package (e.g. 'C:/Python39/python.exe').") | ||
| set(MATERIALX_PYTHON_PYBIND11_DIR "" CACHE PATH | ||
| "Path to a folder containing the PyBind11 source to be used in building MaterialX Python.") | ||
| option(MATERIALX_PYTHON_FORCE_REPLACE_DOCS "Force replace existing docstrings when generating Python binding documentation from Doxygen." OFF) |
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.
Option to allow replacement of docs at build level. Used for wheels and can be used in local or CI builds.
|
@jstone-lucasfilm . All checked in Py* files could be reverted as I've made it so that it can build as part of wheel building. This leaves a "single source of truth" for docs. Either way works (build and check-in) or build only on when building packages. I did not add in a non-wheels route but it would not be hard to do. |
- Cleanup extraction to build better key lookups - Fix insertion to handle statics and globals and use better key lookups.
Purpose
An example of this integrated into VSCode and PyCharm is shown
Changes
Doxygenand then parsing and inserting into theC++wrappers viaa Python script.
DoxygenXML (as input) and matching target C++PyMaterialXpair (as output). It can incrementally update docs from C++ as desired and is not an automated build step.Workflow
The basic logic for build workflow is below. The key is whether doc building is enabled. When it is (and doxygen is available) it will parse and insert comments into the C++. This can be done a local change and checked in. The regular CI does not build docs so there is no effect. Wheels builds will build docs + install doxygen.
graph TB Start([Workflow Start]) Start --> RepoCheck{Repository?} RepoCheck -->|AcademySoftwareFoundation/MaterialX| OfficialRepo[Official Repository] RepoCheck -->|Other Fork| ForkRepo[Fork Repository] %% Regular Build Path OfficialRepo --> RegularBuild[Regular Build Job] ForkRepo --> RegularBuild RegularBuild --> CMakeBuild[CMake Build] CMakeBuild --> DoxygenRegular{MATERIALX_BUILD_DOCS?} DoxygenRegular -->|ON| GenDocsRegular[MaterialXDocs target<br/>Generate Doxygen XML] DoxygenRegular -->|OFF| SkipDocs[No Documentation] GenDocsRegular --> PyBindDocsRegular[PyBindDocs target<br/>pybind_docs.py extracts XML] PyBindDocsRegular --> ForceCheckRegular{MATERIALX_PYTHON_FORCE_REPLACE_DOCS?} ForceCheckRegular -->|ON| ForceReplace[--force flag<br/>Replace existing docs] ForceCheckRegular -->|OFF| NoForce[Update only if newer] ForceReplace --> PyModulesRegular[Build PyMaterialX modules<br/>with embedded docs] NoForce --> PyModulesRegular SkipDocs --> PyModulesNoDoc[Build PyMaterialX modules<br/>without docs] PyModulesRegular --> UploadRegular[Upload Artifact] PyModulesNoDoc --> UploadRegular %% Wheels Build Path OfficialRepo --> SDist[SDist Job] SDist --> WheelsJob[Wheels Job] WheelsJob --> OSSetup{OS-Specific Setup} OSSetup -->|Linux| LinuxDoxy[yum install doxygen] OSSetup -->|MacOS| MacDoxy[brew install doxygen] OSSetup -->|Windows| WinDoxy[choco install doxygen] LinuxDoxy --> BuildWheel[Build Wheel<br/>CMake runs inside cibuildwheel] MacDoxy --> BuildWheel WinDoxy --> BuildWheel BuildWheel --> DoxygenWheel{CMake finds Doxygen?} DoxygenWheel -->|Yes| GenDocsWheel[MaterialXDocs target<br/>Generate Doxygen XML] DoxygenWheel -->|No| WheelNoDocs[Skip docs] GenDocsWheel --> PyBindDocsWheel[PyBindDocs target<br/>pybind_docs.py extracts XML] PyBindDocsWheel --> ForceCheckWheel{FORCE flag?} ForceCheckWheel -->|Set| ForceReplaceWheel[--force flag<br/>Replace existing docs] ForceCheckWheel -->|Not set| NoForceWheel[Update only if newer] ForceReplaceWheel --> PyModulesWheel[Build PyMaterialX modules<br/>with embedded docs] NoForceWheel --> PyModulesWheel WheelNoDocs --> PyModulesWheelNoDoc[Build PyMaterialX modules<br/>without docs] PyModulesWheel --> WheelPackage[Package into wheel] PyModulesWheelNoDoc --> WheelPackage WheelPackage --> UploadWheel[Upload Wheel] %% Styling - only color decision nodes style DoxygenRegular fill:#404040,stroke:#666,stroke-width:2px,color:#fff style DoxygenWheel fill:#404040,stroke:#666,stroke-width:2px,color:#fff style ForceCheckRegular fill:#404040,stroke:#666,stroke-width:2px,color:#fff style ForceCheckWheel fill:#404040,stroke:#666,stroke-width:2px,color:#fff style RepoCheck fill:#404040,stroke:#666,stroke-width:2px,color:#fff style OSSetup fill:#404040,stroke:#666,stroke-width:2px,color:#fff