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

documentation (docstrings) in python #2804

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

pperle
Copy link

@pperle pperle commented May 1, 2020

Description

This pull request adds the docstring from https://carla.readthedocs.io/en/latest/python_api/ into the python source code, ad discussed in #2752. This is still in an early stage, not all references are resolvable, and there is a problem with overloaded functions for which it is not clear which doctrine to choose. Nevertheless, I publish the pull request in order to draw more attention to this topic.

I hope this improves your experience with Carla, you can find a precompiled egg file for Linux (for Carla 0.9.9) here.

Where has this been tested?

  • Platform(s): Linux Ubuntu 18.04.4 LTS
  • Python version(s): 3.5.6
  • Unreal Engine version(s): ...

Possible Drawbacks

This is still very hacky, not all references are resolvable (sometimes this is because of the naming schema and sometimes because there is no documentation). I have only updated the python3 part of BuildPythonAPI.sh because I only have a Linux machine to run Carla and I do not see a reason to support python2 (won't the support be dropped anyway with #2787?).

Please have a look where the documentation doesn't look correct, I guess there are some edge cases I did not cover.


This change is Reviewable

@marcgpuig
Copy link
Contributor

Hi @pperle, that sounds really great!
Which is the advantage of this instead of using Python stubs? These can be also generated from our .yaml docs, I've made some experiments in the past and looks like it can be done, but I've never found the time to properly work on it.

@pperle
Copy link
Author

pperle commented May 7, 2020

There is no advantage in using this instead of Python stubs. This is only the first solution I came up with.

@marcgpuig marcgpuig self-assigned this May 8, 2020
@marcgpuig
Copy link
Contributor

Maybe @bernatx and @nsubiron could join the discussion
My concern is that using stubs should be better since it doesn't require modifying all the Python bindings at all. Your solution could be temporal until we have time to rewrite this system, what do you think?

@germanros1987 germanros1987 requested review from marcgpuig and Axel1092 and removed request for marcgpuig January 26, 2021 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants