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

Set sys.path in wrapping conf.py so custom conf.py does not need to #155

Merged

Conversation

rkent
Copy link
Contributor

@rkent rkent commented Nov 4, 2024

This PR came about because I was trying to figure out why launch did not generate any useful python documentation.

Setting the python directory is tricky for the user, combining both a wrapping directory as well as conf.py directory depth. No real reason they need to do that though, we can do it for them (but they could still override it in their conf.py if they want).

This does not fully solve the problem with launch, as they have another problem in their custom conf.py which I will address there in a PR. But this might help #120

@rkent
Copy link
Contributor Author

rkent commented Nov 8, 2024

As I am looking at packages that are struggling with rosdoc2, I cam across another reason why this PR is important. The recommended way at the moment to set the python path in conf.py is with a complicated statement like:

sys.path.insert(0, os.path.abspath(os.path.join('..', '..', '..', '..', 'cv_bridge', 'python')))

The problem though is that this depends on where rosdoc2 is run from. If you run it from the repo, or you run it from the package directory itself, changes what the above should be. That is clearly not something that is acceptable, so this needs to be set in the wrapping conf.py (as in this PR), not the user conf.py

@rkent
Copy link
Contributor Author

rkent commented Nov 8, 2024

After more thinking about the sys.path issue in conf.py, I realized that there really is no reliable way to do that in conf.py. So I changed the code comment in a followup commit that discourages setting it there.

Copy link
Member

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

Thanks this looks like a good way to avoid requiring user customization.

@tfoote tfoote merged commit 5b4acb6 into ros-infrastructure:main Nov 8, 2024
5 checks passed
@rkent rkent deleted the always-append-python-src-directory branch December 10, 2024 16:24
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.

2 participants