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 python3-typeshed #40651

Merged
merged 9 commits into from
Apr 19, 2024
Merged

Add python3-typeshed #40651

merged 9 commits into from
Apr 19, 2024

Conversation

InvincibleRMC
Copy link
Contributor

@InvincibleRMC InvincibleRMC commented Apr 17, 2024

Please add the following dependency to the rosdep database.

Package name:

python3-typeshed

Package Upstream Source:

https://github.com/python/typeshed

Purpose of using this:

In the process of accomplishing ros2/rclpy#1257 we need the types for pyyaml being used for mypy support.

The source is here:

https://packages.ubuntu.com/jammy/amd64/python/python3-typeshed
https://packages.debian.org/search?suite=default&section=all&arch=any&searchon=names&keywords=python3-typeshed

@InvincibleRMC InvincibleRMC requested a review from a team as a code owner April 17, 2024 03:06
@github-actions github-actions bot added the rosdep Issue/PR is for a rosdep key label Apr 17, 2024
@InvincibleRMC InvincibleRMC changed the title Update python.yaml Add types-PyYaml Apr 17, 2024
Copy link
Contributor

@sloretz sloretz left a comment

Choose a reason for hiding this comment

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

Since this is a pip only key we won't be able to make rclpy (or any package released to the ROS build farm) depend on it. Does it still work for your purposes?

@InvincibleRMC
Copy link
Contributor Author

InvincibleRMC commented Apr 17, 2024

Ah ok. I should be able to switch it to the python3-typeshed apt package. However that will install slighty more stuff and only exist on Ubuntu 22+.

@InvincibleRMC InvincibleRMC changed the title Add types-PyYaml Add python3-typeshed Apr 17, 2024
@cottsay
Copy link
Member

cottsay commented Apr 18, 2024

It looks like mypy on Debian, Ubuntu, and Fedora doesn't actually use a discrete "typeshed" package even if available, and rather bundles typeshed as mypy.typeshed. Would it be possible to use that instead, since your long-term goal is to use mypy anyway?

@InvincibleRMC
Copy link
Contributor Author

The CI is failing because python3-typeshed doesn't exist on Ubuntu 20. Should I disable it for that older version? And if so how?

@InvincibleRMC
Copy link
Contributor Author

It looks like mypy on Debian, Ubuntu, and Fedora doesn't actually use a discrete "typeshed" package even if available, and rather bundles typeshed as mypy.typeshed. Would it be possible to use that instead, since your long-term goal is to use mypy anyway?

From what I can tell mypy.typeshed only has stdlib types and not typeshed's stubs folders. From testing this without installing python3-typeshed or types-pyyaml mypy raises the following.

src/rclpy/rclpy/rclpy/parameter.py:27: error: Library stubs not installed for "yaml"  [import-untyped]
src/rclpy/rclpy/rclpy/parameter.py:27: note: Hint: "python3 -m pip install types-PyYAML"

@cottsay
Copy link
Member

cottsay commented Apr 18, 2024

From what I can tell mypy.typeshed only has stdlib types and not typeshed's stubs folders.

Thanks for checking.

Is my understanding correct in saying that your goal is to have the PyYAML stubs? I'm asking mainly because there is no python3-typeshed package available for RHEL 9, which is a tier-2 platform for ROS 2. However, it does appear that the individual stubs are packaged.

I think it would be appropriate to go back to the python3-types-pyyaml key you had before, but add rules for Debian/Ubuntu for python3-typeshed under it. Then you could add python3-types-pyyaml rules for Fedora and RHEL as well. A rosdep "key" really represents a resource, so if the python3-typeshed deb provides what you'd expect for python3-types-pyyaml, then it should be okay (even though you get a little more than you needed).

There is precedent for this pattern in other packages in rosdep. Different distributions make different decisions regarding where to draw lines when separating subpackages. Boost is another example of where major distributions make different decisions. It's unfortunate that there is no Ubuntu package specifically for the PyYAML types because that's typically how we derive the name of the rosdep key, but I think python3-types-pyyaml would probably be the package name if it did exist.

rosdep/python.yaml Outdated Show resolved Hide resolved
rosdep/python.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@sloretz sloretz left a comment

Choose a reason for hiding this comment

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

@sloretz sloretz merged commit ea0495a into ros:master Apr 19, 2024
4 checks passed
@InvincibleRMC InvincibleRMC deleted the type-pyyaml branch April 19, 2024 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rosdep Issue/PR is for a rosdep key
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants