Skip to content

Static typing for Message, Services, and Actions #206

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

Open
wants to merge 67 commits into
base: rolling
Choose a base branch
from

Conversation

InvincibleRMC
Copy link
Contributor

@InvincibleRMC InvincibleRMC commented Mar 15, 2024

Adds static typing to generated messages. Closes #157.

Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
@InvincibleRMC InvincibleRMC marked this pull request as draft March 15, 2024 07:12
@InvincibleRMC
Copy link
Contributor Author

Still need to fix a circular import issue.

Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
@InvincibleRMC InvincibleRMC marked this pull request as ready for review March 15, 2024 16:35
InvincibleRMC and others added 18 commits March 15, 2024 13:36
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
@InvincibleRMC
Copy link
Contributor Author

One thing I have learn which is relevant to #216 is that currently according to pep 484 is that a type annotation of float allows int. There is currently no defined way to define a realFloat type that disallows int. There are many discussions about this.

@InvincibleRMC
Copy link
Contributor Author

@sloretz @fujitatomoya could you review this pull request if you get the chance? It would improve the pub/sub, service/clients in rclpy since currently the generics are being ignored.

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.

Thank you for the PR! It's no small feat to make changes to the empy templates 🙇

I'm still looking through the changes in _msg.py.em, but I have some feedback in the meantime.

InvincibleRMC and others added 5 commits March 13, 2025 19:54
Co-authored-by: Shane Loretz <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Co-authored-by: Shane Loretz <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
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.

Few more places to use if @(member.name) is not None else instead of or

Signed-off-by: Michael Carlstrom <[email protected]>
@InvincibleRMC
Copy link
Contributor Author

@sloretz Following up I was able to remove the duplicate imports. Let me know if there are any other questions/suggestions. Thanks again for reviewing this pr.

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.

Python type annotation
3 participants