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

Create py.typed to mark this library as typed #379

Merged
merged 2 commits into from Aug 28, 2023

Conversation

ottojo
Copy link
Contributor

@ottojo ottojo commented Aug 25, 2023

This marker file informs type checkers of downstream libraries that launch_ros is typed.
In my use case, this enables using the strict typechecking mode in pyright for a downstream project using launch_ros

This was done for rclpy in ros2/rclpy#946

@ottojo
Copy link
Contributor Author

ottojo commented Aug 25, 2023

I'm not quite sure how that caused the failed test in the Rpr__launch_ros__ubuntu_jammy_amd64 CI, that might be unrelated?

Apropos CI: In the linked rclpy PR there is some discussion on testing. Pyright can test libraries for type correctness, but this test currently fails for launch_ros:

pyright --verifytypes launch_ros --ignoreexternal

Symbols exported by "launch_ros": 265
  With known type: 148
  With ambiguous type: 5
  With unknown type: 112
    (Ignoring unknown types imported from other packages)
  Functions without docstring: 13
  Functions without default param: 0
  Classes without docstring: 0

Other symbols referenced but not exported by "launch_ros": 0
  With known type: 0
  With ambiguous type: 0
  With unknown type: 0

Type completeness score: 55.8%

It does however also fail for rclpy:

pyright --verifytypes rclpy --ignoreexternal

[...]

Symbols exported by "rclpy": 827
  With known type: 228
  With ambiguous type: 13
  With unknown type: 586
    (Ignoring unknown types imported from other packages)
  Functions without docstring: 118
  Functions without default param: 0
  Classes without docstring: 27

Other symbols referenced but not exported by "rclpy": 19
  With known type: 6
  With ambiguous type: 0
  With unknown type: 13

Type completeness score: 27.6%

I think it still makes sense to mark this as "supposed to by typed" (if this is the intention, i assume this based on the large amount of type hints), it works well for rclpy.

edit: for reference: the pyright documentation for libraries: https://microsoft.github.io/pyright/#/typed-libraries?id=typing-guidance-for-python-libraries

@clalancette
Copy link
Contributor

I'm not quite sure how that caused the failed test in the Rpr__launch_ros__ubuntu_jammy_amd64 CI, that might be unrelated?

Yeah, it's unrelated. It should be fixed by #377

launch_ros/setup.py Outdated Show resolved Hide resolved
launch_ros/launch_ros/py.typed Outdated Show resolved Hide resolved
Signed-off-by: Jonas Otto <jonas@jonasotto.com>
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Thanks for iterating! This looks good to me, I'll run CI on it next.

@clalancette
Copy link
Contributor

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

launch_ros/setup.py Outdated Show resolved Hide resolved
Signed-off-by: Jonas Otto <jonas@jonasotto.com>
@clalancette
Copy link
Contributor

New CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@clalancette
Copy link
Contributor

Looks good, thanks for iterating!

@clalancette clalancette merged commit 760e219 into ros2:rolling Aug 28, 2023
3 checks passed
@ottojo ottojo deleted the py-typed-marker branch August 29, 2023 08:19
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

2 participants