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

add python docs theme as a dependency #2645

Merged
merged 4 commits into from
May 4, 2021

Conversation

PeterMitrano
Copy link
Contributor

Description

In #2547 we use the RTD theme for python, but didn't add it as a dependency, which is causing the docs job to not generate python docs

rosdep does not look at doc_depend which might cause problems. I'm not sure how to trigger a build farm job to test this, but if that's a problem we can change it to a normal depend.

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference
  • Document API changes relevant to the user in the MIGRATION.md notes
  • Create tests, which fail without this PR reference
  • Include a screenshot if changing a GUI
  • While waiting for someone to review your request, please help review another open pull request to support the maintainers

@codecov
Copy link

codecov bot commented May 3, 2021

Codecov Report

Merging #2645 (a79185b) into master (005de1f) will decrease coverage by 0.09%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2645      +/-   ##
==========================================
- Coverage   60.58%   60.50%   -0.08%     
==========================================
  Files         402      402              
  Lines       29619    29619              
==========================================
- Hits        17941    17917      -24     
- Misses      11678    11702      +24     
Impacted Files Coverage Δ
.../ompl_interface/src/detail/constrained_sampler.cpp 43.25% <0.00%> (-16.21%) ⬇️
...e/src/parameterization/model_based_state_space.cpp 68.31% <0.00%> (-2.81%) ⬇️
moveit_core/robot_model/src/joint_model_group.cpp 63.06% <0.00%> (-1.94%) ⬇️
...meterization/work_space/pose_model_state_space.cpp 81.42% <0.00%> (-1.92%) ⬇️
...on/pick_place/src/approach_and_translate_stage.cpp 73.24% <0.00%> (-0.70%) ⬇️
...dl_kinematics_plugin/src/kdl_kinematics_plugin.cpp 81.00% <0.00%> (-0.41%) ⬇️
...raint_samplers/src/default_constraint_samplers.cpp 83.89% <0.00%> (-0.32%) ⬇️
moveit_core/robot_state/src/robot_state.cpp 50.77% <0.00%> (-0.09%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 005de1f...a79185b. Read the comment docs.

Copy link
Contributor

@rhaschke rhaschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rhaschke rhaschke merged commit 9bc4b89 into moveit:master May 4, 2021
@rhaschke
Copy link
Contributor

rhaschke commented May 4, 2021

@cottsay (I guess you are a build-farm maintainer): How often are doc jobs triggered on the build farm?
Could you maybe trigger this job manually? Thanks.

@cottsay
Copy link

cottsay commented May 4, 2021

It's triggered automatically whenever https://github.com/ros-planning/moveit/ is changed.

This change won't trigger that job because you merged it to the master branch, which is not the configured branch for Noetic: https://github.com/ros/rosdistro/blob/69b57cb488ec08ee0524a296a5cdff919520e0c2/noetic/distribution.yaml#L3897-L3901

@rhaschke
Copy link
Contributor

rhaschke commented May 4, 2021

Thanks for this hint. I will forward noetic-devel to master...

@rhaschke
Copy link
Contributor

rhaschke commented May 6, 2021

@cottsay, the build farm still didn't trigger a new doc job, although the noetic-devel branch was updated two days ago.
What's the frequency of SCM checks?

@PeterMitrano
Copy link
Contributor Author

cross referencing #2606

@PeterMitrano
Copy link
Contributor Author

Progress! The install worked, and http://docs.ros.org/en/noetic/api/moveit_core/html/python/ is live. But there are still errors

Here's the log:
https://build.ros.org/view/Ndoc/job/Ndoc__moveit__ubuntu_focal_amd64/16/consoleFull#console-section-24

Here are the errors/warnings

[autosummary] failed to import 'moveit.core': no module named moveit.core
api.rst:1: WARNING: Error in "autosummary" directive:
html_static_path entry '/tmp/ws/src/moveit/moveit_core/doc/_static' does not exist

So when rosdoc_lite calls the docs, perhaps it isn't running from a correctly setup environment in terms of python path or something? Because import moveit.core should work.

The second warning could be because sphinx is out of date. Not sure what version it uses, perhaps https://github.com/ros-infrastructure/catkin-sphinx ? which is definitely out of date...

The last error might be caused by the first two. Not sure.

Also not sure how to proceed with debugging, because these are issues that depend on me knowing things about the build farm config/infrastructure. Does anyone else have suggestions on how to proceed?

@PeterMitrano
Copy link
Contributor Author

It seems the autosummary extension has been in sphinx since version 0.6, and it seems the version on the build server is 1.8.5 so nto sure what's going on there.

I also can't tell if the install workspace is sourced when running rosdoc_lite. If not, I think it needs to be in order for the import moveit.core to work, which is needed for the docs to generate.

@rhaschke
Copy link
Contributor

rhaschke commented May 7, 2021

@cottsay, who can help on this doc generation issue?
Obviously, the build farm doesn't build the packages before calling rosdoc_lite. In our case, this results in sphinx not finding the actual python packages, because they are built with pybind11. Is there a way to indicate that the repo needs a build before the docs are generated?

@PeterMitrano
Copy link
Contributor Author

interestingly it does look like it is building the workspace, I can see it in the log. Everything gets installed to some temp install_isolated location. But I don't see that install space getting "sourced" before/during the rosdoc_lite steps

@cottsay
Copy link

cottsay commented May 7, 2021

@cottsay, the build farm still didn't trigger a new doc job, although the noetic-devel branch was updated two days ago.
What's the frequency of SCM checks?

Three days: https://github.com/ros-infrastructure/ros_buildfarm/blob/f15b1c39f0f2126bcc1bf0a32f65db9fa57d125a/ros_buildfarm/templates/doc/doc_job.xml.em#L66-L69

Is there a way to indicate that the repo needs a build before the docs are generated?

I have no clue. @tfoote?

@tfoote
Copy link
Contributor

tfoote commented May 8, 2021

Is there a way to indicate that the repo needs a build before the docs are generated?

There's no way to do that. rosdoc_lite runs a custom pseudo build to generate messages but doesn't build the package itself. Requiring all the build dependencies management is a lot more overhead too.

@PeterMitrano
Copy link
Contributor Author

Sounds like that's something that's not easily changed, and something we might not want to change in general. But without that it seems we can't use ros docs for these python docs.

We could generate docs on some other server and host it manually, I guess?

Or it might be possible to do the generation of the .rst files before hand, so it doesn't have to import moveit.core on the build farm. Not sure how that would work, but then it could still go through the normal rosdocs pipeline?

@PeterMitrano
Copy link
Contributor Author

Here's a start of a possible workaround:

https://github.com/UM-ARM-Lab/moveit/tree/include-generated-python-docs/moveit_core/doc

Basically I just ran rosdoc myself to generate the *.rst files manually, then changes the config so it just reads existing one and doesn't try to import or use autosummary. The obvious downside is that we have to run this manually in order to update the generated docs

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

Successfully merging this pull request may close these issues.

None yet

4 participants