-
Notifications
You must be signed in to change notification settings - Fork 804
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
Add docstring generation for Python wrapper #2038
base: develop
Are you sure you want to change the base?
Conversation
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.
Cool! Some issues:
- indeed, wrap is pulled in from a different repo. Maybe @varunagrawal can comment on correct way to change wrap and get it back into GTSAM.
- that also affects the flag names: wrap is not GTSAM, so cmake variables there have a different naming convention I think
- Finally, is it possible to not need GTSAM_PYTHON_DOCS_XML_SOURCE?
Yes, it is now changed to look for xml in {project_root}/xml. Regarding the other two points:
I await @varunagrawal for confirmation on (1) and insight on (2). |
This is pretty interesting. I wonder why this isn't just a Git submodule? Then, the |
@yambati03 We opted to make |
@p-zach check out https://github.com/borglab/wrap. You'll want to move your changes over there so that they don't get overwritten the next time we pull that subtree in. Let me know if you need any help with understanding that repo. Happy to connect offline. |
I wouldn't say Github tarbulls don't work, but they need some extra work. Lots of people have gotten this to work with submodules. |
Also FYI, there are a lot of subfolders in wrap that were originally intended for documentation generation but I guess they were never completed? If @p-zach's updates work, we should nuke those others and clean up the repository as general housekeeping. |
Adds the capability to extract Doxygen documentation from the C++ source code and insert these docs into the Python bindings. This means that the documentation is then available in the
help()
function. The functionality can be seen in these examples:To generate these Python docstrings, follow these steps:
build/<build-name>/doc
, but it might still work if you use thedoc/Doxyfile.in
(if, for example, you haven't built GTSAM yet and don't want to build it just to compile the Doxyfile).GTSAM_BUILD_PYTHON_DOCS_FROM_XML
to ON.GTSAM_PYTHON_DOCS_XML_SOURCE
to the path to thexml
folder generated by Doxygen.pip install .
it.The parser is fairly robust but not perfect. I encourage anybody to test it out on functions that have well-formatted documentation in the source code so that we can improve it (naturally, the parser can only work with what's in the source--bad or poorly formatted source docs --> bad or poorly formatted Python docs).
To build these docstrings automatically with the pypi package that @yambati03 is working on, the build process will need to be amended so that Doxygen XML is generated before GTSAM Python is built. It takes about 3 minutes to generate the Doxygen files on my machine, and that can probably be shortened if necessary by editing the Doxyfile to not build unnecessary files.
Lastly--this PR edits
wrap
. Does that mean the changes need to be duplicated in thewrap
repo? Not sure what the practice is there.