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

Resolve TODO sections in README.md regarding C++ and Python API documentation #58

Open
berndpfrommer opened this issue Feb 16, 2024 · 16 comments

Comments

@berndpfrommer
Copy link

I tried to document my package using rosdoc2, but alas, the sections of the rosdoc2 README file have "TODO" entries exactly where it gets interesting. I basically want to have a custom landing page with links to auto-generated C++ API and Python API documentation. In fact I have a pybind11 (C++/Python) code base that I'd like to get working as well. If you don't have time to write documentation, could you please provide a link to a working example in place of the "TODO" entries. Much appreciated!

PS: I would offer to help filling in the documentation blanks but it really takes a while to get the knack of these document generators. I couldn't get much working without cook-book style instructions.

@ottojo
Copy link
Contributor

ottojo commented Feb 16, 2024

I was in the very same position just a few hours ago, and wrote down some notes on my blog, perhaps they are of use to you! (I will see if i can add this in a nicer form to the official docs perhaps?)

@berndpfrommer
Copy link
Author

@ottojo Just followed your blog and it's nothing short of awesome. Worked first shot for me and will be useful for many others who are in the same situation: have a README.md and would like to see that show up in a nicer formatted form together with links to the API documentation.
I don't think you want to automate that any more (as you suggest in your blog). It's almost more useful in its raw form (as described on your blog) since it exposes some of the underlying mechanics to people so they can customize it to their liking. I would wait with wrapping it into an automated tool and first document the pedestrian way until you see what the full usage pattern is. Then wrap the common cases into a tool.

Alas following your blog I only get C++ API generation, but since my package is a pybind11 wrapper, the generated C++ docs are useless, I really need the python API. Now I have to figure out how to extract that from pybind11. Your blog gave me a good starting point for that. Thanks again!

@Yadunund
Copy link
Collaborator

Hi @berndpfrommer,

We should definitely update the README to replace those TODO statements (all except Packages with ROS 2 Messages and other kinds of Interfaces since this feature is still pending).

In the meanwhile, I hope you can get an idea of how to setup rosdoc2 to generate documentation for cpp, python, and cpp+python (pybind11) packages by looking at setup from other ROS 2 repositories.

Let's consider the Cpp + Python package use case. rclpy is a good example since it is an ament_cmake package that needs documentation generated for its python modules. The documentation is available here for reference. (You can lookup a package on index.ros.org and click on the API docs button on the top right to view its documentation)

  • Here's the rosdoc2.yaml file for this package.
  • The critical settings are python_source which tells rosdoc2 where the python modules are located relative to the package.xml file and most importantly override_build_type which forces rosdoc2 to run the builder as if this were a python package (skip doxygen generation, breathe, etc).

Another relevant example is launch

  • The generated documentation can be seen here. Notably, there is a "custom landing page" with links to an explanation of the architecture.
  • The custom landing page is courtesy of the setup within the doc folder. rosdoc2 implictly checks for this doc folder.

@berndpfrommer
Copy link
Author

@Yadunund thank you very much for the pointers. They fill in the gaps left from @ottojo 's blog and in combination with the tricks from @ottojo 's blog I get very satisfying results, at least when I locally build with rosdoc2. Why the page at docs.ros.org is not updating I don't quite understand. I'll probably have to release the package again.
Feel free to close this issue or leave it open until the TODO's have been resolved!

@Yadunund
Copy link
Collaborator

@Yadunund thank you very much for the pointers. They fill in the gaps left from @ottojo 's blog and in combination with the tricks from @ottojo 's blog I get very satisfying results, at least when I locally build with rosdoc2. Why the page at docs.ros.org is not updating I don't quite understand. I'll probably have to release the package again. Feel free to close this issue or leave it open until the TODO's have been resolved!

I assume your package already has an entry in a distributiion.yaml file in rosdistro? And that it has a doc config like this? It might be worth checking if there were any issues in the Doc job the buuldfarm ran for your package. https://build.ros2.org/view/Rdoc/ (switch to Hdoc/Idoc to check Humble/Iron job). Let me check if we poll for changes to run the Doc job or whether a release is required.

@berndpfrommer
Copy link
Author

Yes, the package has a doc entry in distribution.yaml, and I see it actually rebuilt the docs on Feb 23rd so that is working, thanks again for the pointers!

However, an error occurs that I don't see when building locally:

06:14:25 WARNING: autodoc: failed to import module 'event_camera_py'; the following exception was raised:
06:14:25 No module named 'event_camera_py'

I think that is why the Python API documentation is not generated (that works when running rosdoc2 locally).
The other wrinkle: the landing page on rosindex is just a translated (and somewhat garbled) version of the README.md, without the direct links to the Python API. Ideally I'd have this page show up when searching the rosindex.
But we are getting further away now from this issue's topic, so I should probably open another one for this.

@Yadunund
Copy link
Collaborator

Yadunund commented Feb 26, 2024

Glad to head that the doc job rebuilt.

Regarding the import errors, this usually happens when autodoc runs all the import commands in the module (i think to provide cross-linked references to other modules), and if your environment is not sourced, the required packages may not be in the python path.
By default rosdoc2 is setup such that it tells autodoc to ignore imports of all modules from packages listed as <exec_depends> in the package.xml. See logic here. The upside is that we don't require any packages to be built to run the doc job. The downside is that we don't have cross-linked references.
To have full cross-linking, rosdoc2 can be run after sourcing a workspace where the imported modules are built. This is not what the buildfarm does which instead relies on the default behavior.

I'm curious if when you run rosdoc2 locally, you have a workspace sourced? That would explain the difference from the the buildfarm job. If so, do you trying the command without any workspace sourced and after checking if the <exec_depends> are correctly defined in your package?

The rosindex issue is definitely worth a separate ticket.

@berndpfrommer
Copy link
Author

It is as you expected: if I don't source my local workspace, I get the error below, if I source it, the error disappears:

WARNING: autodoc: failed to import module 'event_camera_py'; the following exception was raised:         
No module named 'event_camera_py'

I checked my <exec_depends> and found one missing dependency (rpyutils) but adding that dependency into package.xml made no difference. It is pretty clearly saying that event_camera_py is missing. I don't see how I can get the API document correctly built on the buildfarm. That must be possible somehow because other packages do it.

@Yadunund
Copy link
Collaborator

Yadunund commented Mar 3, 2024

Is event_camera_py the package for which the documentation is being generated?

@berndpfrommer
Copy link
Author

Yes, event_camera_py is the package name as well as the name of the python module for which documentation is supposed to be generated. I tried inserting an <exec_depend> on event_camera_py into package.xml but it gave me a build error saying that is not allowed. Makes sense.

@Yadunund
Copy link
Collaborator

Yadunund commented Mar 3, 2024

I see thanks for that information. You could rely on <doc_depend> like so.

@berndpfrommer
Copy link
Author

If I put this line into package.xml:

 <doc_depend>event_camera_py</doc_depend>

I get this error:

rosdoc2 build --package-path ./src/event_camera_py
Failed to parse potential ROS package manifest in './src/event_camera_py': Error(s) in package './src/event_camera_py/package.xml':
The package "event_camera_py" must not "doc_depend" on a package with the same name as this package

It doesn't look to me like it would depend on any other modules except maybe on rclpy, which I also tried but it makes no difference, whether I doc_depend or exec_depend on it.

Lastly, I ran strace to see if during the rosdoc2 compilation I can see what modules it is trying to pull in, but it's far from clear from the output. The only suspicious thing was that it failed to find a file (it looked in various locations, but this was the last location):

/usr/share/locale-langpack/en/LC_MESSAGES/event_camera_py.mo

Not sure if that causes the import to fail.

@ottojo
Copy link
Contributor

ottojo commented Mar 6, 2024

@berndpfrommer maybe you are not configuring the python module search path correctly in spinx conf.py.

rclpy adds sys.path.insert(0, os.path.abspath('.')) here: https://github.com/ros2/rclpy/blob/rolling/rclpy/docs/source/conf.py#L31

and the autogenerated conf.py from rosdoc2 even resolves the absolute package path during generation: sys.path.insert(0, os.path.abspath(os.path.join('{package_src_directory}', '..'))) https://github.com/ros-infrastructure/rosdoc2/blob/main/rosdoc2/verbs/build/builders/sphinx_builder.py#L211

edit: Since rosdoc2 wraps the custom conf.py with more configuration from a script which will be placed in the package root directory during building, . will refer to the package root during and not the doc directory.

@berndpfrommer
Copy link
Author

@ottojo That may really be it, and in fact I pushed some changes along those lines yesterday night but the doc job run is not triggered for some reason: https://build.ros2.org/job/Idoc__event_camera_py__ubuntu_jammy_amd64/

@berndpfrommer
Copy link
Author

Meanwhile the job has run (whatever triggered it I don't know) but just adding the line sys.path.insert(0, os.path.abspath('.')) did not do the trick. I get the same error.
The only avenue I see left now is to start with the rclpy package and gradually morph it into my event_camera_py package, then see where things break. Regrettably this is more effort than I wanted to put in so I'll postpone this for now.

@ottojo
Copy link
Contributor

ottojo commented Mar 9, 2024

Oh, i did not realize that the python module in event_camera_py is fully built using pybind11. As far as i can tell, even rclpy only has documentation for the python interface which is actually implemented in python. I don't know any package which documents python modules implemented in C++.

To do this, it would probably be necessary to build the package (creating the python module) before running sphinx (which must import the python package, which doesnt exist without building the c++ code...). I don' t know if the documentation CI job has a way to do this already...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants