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

Include py.typed in a package #936

Closed
bonprosoft opened this issue May 3, 2022 · 5 comments · Fixed by #946
Closed

Include py.typed in a package #936

bonprosoft opened this issue May 3, 2022 · 5 comments · Fixed by #946
Assignees

Comments

@bonprosoft
Copy link
Contributor

Feature request

Feature description

I use mypy, one of the most famous static type checkers in Python, to check my Python code depending on rclpy.
However, mypy won't use the type annotation of rclpy, since mypy only uses the type annotation of packages that are marked as PEP561-compatible (i.e. Packages that have py.typed inside).

Therefore, I can't check the code like:

$ cat foo.py
import rclpy

reveal_type(rclpy)
reveal_type(rclpy.init)

rclpy.foo  # should be error
rclpy.init(foo="bar")  # should be error

$ mypy foo.py 
foo.py:1: error: Cannot find implementation or library stub for module named 'rclpy'
foo.py:1: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
foo.py:3: note: Revealed type is 'Any'
foo.py:4: note: Revealed type is 'Any'
Found 1 error in 1 file (checked 1 source file)

Would it be possible to include py.typed in rclpy package? (I am happy to work on this task!)

Implementation considerations

@adityapande-1995
Copy link

I tried adding py..typed to the rclpy directory, but I'm still seeing the same output ;

 0% ❯ mypy foo.py
foo.py:1: error: Cannot find implementation or library stub for module named "rclpy"
foo.py:1: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
foo.py:3: note: Revealed type is "Any"
foo.py:4: note: Revealed type is "Any"
Found 1 error in 1 file (checked 1 source file)

I have a file called py.typed in ~/ros2_rolling/install/rclpy/lib/python3.8/site-packages/rclpy , where the rclpy module is located (using rclpy.__file__)

@bonprosoft
Copy link
Contributor Author

bonprosoft commented May 21, 2022

Thank you the response!

but I'm still seeing the same output

This is because mypy uses site.getsitepackages, where PYTHONPATH doesn't affect, to look up Python stub packages.
https://github.com/python/mypy/blob/f19a711eae747e2e5525bf0b0baaa3d3ae972fbd/mypy/modulefinder.py#L759-L760
https://github.com/python/mypy/blob/f19a711eae747e2e5525bf0b0baaa3d3ae972fbd/mypy/pyinfo.py#L25-L36

Here is the document:
https://mypy.readthedocs.io/en/stable/running_mypy.html#how-imports-are-found

The easiest way to try mypy for the example is to specify PYTHONUSERBASE:

$ PYTHONUSERBASE=/opt/ros/foxy mypy foo.py
foo.py:3: note: Revealed type is "types.ModuleType"
foo.py:4: note: Revealed type is "def (*, args: Union[builtins.list[builtins.str], None] =, context: Union[rclpy.context.Context, None] =)"
foo.py:6: error: Module has no attribute "foo"
foo.py:7: error: Unexpected keyword argument "foo" for "init"
/opt/ros/foxy/lib/python3.8/site-packages/rclpy/__init__.py:62: note: "init" defined here
Found 2 errors in 1 file (checked 1 source file)

If you use python in a virtualenv, the above method may not work since site package would set ENABLE_USER_SITE=False in some environments:
https://github.com/python/cpython/blob/5b71b519f966e1017c868ea2b27c61a5eac38c1f/Lib/site.py#L533-L539

So, general solution for ROS packages might be creating a .pth file in one of the site-packages as follows:

$ echo "/opt/ros/foxy/lib/python3.8/site-packages" > $(python -c "import site; print(site.getsitepackages()[0])")/ros.pth
$ mypy foo.py

@adityapande-1995
Copy link

Thanks for the clarification @bonprosoft ! Would you be willing to open a PR to fix this in rclpy ? If you tag me as a reviewer I could take a look at it.

@bonprosoft
Copy link
Contributor Author

Of course! I've created a PR #946. Could you please take a look?

@bonprosoft
Copy link
Contributor Author

Also, I have a question related to the type annotations.

  • What do you think about the static type checker support of ROS packages? For example, can I send a PR to add more type annotations or fix the existing wrong type annotations of rclpy in the future?
    • Maybe I can introduce some tests for type annotations itself (e.g. stubtest) if it is OK.

Just FYI: I think the following feature request is also good to have and I am ready to contribute.
I know that you are busy, but it would be appreciated if I could get any feedback:

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 a pull request may close this issue.

2 participants