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

Python type annotation #157

Open
MrBlenny opened this issue Apr 19, 2022 · 7 comments · May be fixed by #205 or #206
Open

Python type annotation #157

MrBlenny opened this issue Apr 19, 2022 · 7 comments · May be fixed by #205 or #206
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@MrBlenny
Copy link

MrBlenny commented Apr 19, 2022

Feature request

Feature description

It would be useful if the python classes included type annotations.

For example:

Parent.msg

Child[] children
Child child

Child.msg

uint8 some_child_content

Would generate something like:

+from perception_msgs.msg import Child

class Parent(metaclass=Metaclass_Parent):
    """Message class 'Parent'."""

    __slots__ = [
        '_children',
        '_child',
    ]

    _fields_and_field_types = {
        'children': 'sequence<perception_msgs/Child>',
        'child': 'perception_msgs/Child',
    }

    SLOT_TYPES = (
        rosidl_parser.definition.UnboundedSequence(rosidl_parser.definition.NamespacedType(['perception_msgs', 'msg'], 'Child')),  # noqa: E501
        rosidl_parser.definition.NamespacedType(['perception_msgs', 'msg'], 'Child'),  # noqa: E501
    )

-   def __init__(self, **kwargs):
+   def __init__(self, children: Optional[List[Child]]=None, child: Optional[Child]=None):
-       assert all('_' + key in self.__slots__ for key in kwargs.keys()), \
-           'Invalid arguments passed to constructor: %s' % \
-           ', '.join(sorted(k for k in kwargs.keys() if '_' + k not in self.__slots__))
-       self.children = kwargs.get('children', [])
-       from perception_msgs.msg import Child
-       self.child = kwargs.get('child', Child())
+       self.children = children or []
+       self.child = child or Child()

    def __repr__(self):
        typename = self.__class__.__module__.split('.')
        typename.pop()
        typename.append(self.__class__.__name__)
        args = []
        for s, t in zip(self.__slots__, self.SLOT_TYPES):
            field = getattr(self, s)
            fieldstr = repr(field)
            # We use Python array type for fields that can be directly stored
            # in them, and "normal" sequences for everything else.  If it is
            # a type that we store in an array, strip off the 'array' portion.
            if (
                isinstance(t, rosidl_parser.definition.AbstractSequence) and
                isinstance(t.value_type, rosidl_parser.definition.BasicType) and
                t.value_type.typename in ['float', 'double', 'int8', 'uint8', 'int16', 'uint16', 'int32', 'uint32', 'int64', 'uint64']
            ):
                if len(field) == 0:
                    fieldstr = '[]'
                else:
                    assert fieldstr.startswith('array(')
                    prefix = "array('X', "
                    suffix = ')'
                    fieldstr = fieldstr[len(prefix):-len(suffix)]
            args.append(s[1:] + '=' + fieldstr)
        return '%s(%s)' % ('.'.join(typename), ', '.join(args))

    def __eq__(self, other):
        if not isinstance(other, self.__class__):
            return False
        if self.children != other.children:
            return False
        if self.child != other.child:
            return False
        return True

    @classmethod
    def get_fields_and_field_types(cls):
        from copy import copy
        return copy(cls._fields_and_field_types)

    @property
-   def children(self):
+   def children(self) -> List[Child]:
        """Message field 'children'."""
        return self._children

    @children.setter
-   def children(self, value):
+   def children(self, value: List[Child]):
        if __debug__:
            from perception_msgs.msg import Child
            from collections.abc import Sequence
            from collections.abc import Set
            from collections import UserList
            from collections import UserString
            assert \
                ((isinstance(value, Sequence) or
                  isinstance(value, Set) or
                  isinstance(value, UserList)) and
                 not isinstance(value, str) and
                 not isinstance(value, UserString) and
                 all(isinstance(v, Child) for v in value) and
                 True), \
                "The 'children' field must be a set or sequence and each value of type 'Child'"
        self._children = value

    @property
-   def child(self):
+   def child(self) -> Child: 
        """Message field 'child'."""
        return self._child

    @child.setter
-   def child(self, value):
+   def child(self, value: Child):    
        if __debug__:
            from perception_msgs.msg import Child
            assert \
                isinstance(value, Child), \
                "The 'child' field must be a sub message of type 'Child'"
        self._child = value

Implementation considerations

  • There seems to be an issue with typing @property getters/setters. For me, using pylance the above syntax thinks child is of type property rather than type Child
  • Backwards compatibility requirements - do we need to support python < 3.8? I assume type annotations break old python.
  • Another path.... from dataclasses import dataclass gives us most of these features and more. Plus, it can be used with other libraries like dacite
@clalancette
Copy link
Contributor

  • Backwards compatibility requirements - do we need to support python < 3.8? I assume type annotations break old python.

For now, yes. We need to support Python 3.6 for RHEL-8 compatibility.

@clalancette clalancette added enhancement New feature or request help wanted Extra attention is needed labels Apr 19, 2022
@russkel
Copy link

russkel commented Apr 20, 2022

What if separate stubs (pyi files, I think) were generated, leaving the actual source code alone?

@bonprosoft
Copy link

Hello.
I implemented rosidl_generator_mypy, which generates .pyi file for each message/service/action file.

Could you try this repository if you are interested in?

As I think it's good to have the feature in this repository, I haven't published this repository to rosdistro, yet.
If it's okay, I am happy to create a pull request to this repository to implement the stub generator feature!

@russkel
Copy link

russkel commented May 3, 2022

Hi @bonprosoft, thanks for that, nice work.

As I think it's good to have the feature in this repository, I haven't published this repository to rosdistro, yet.
If it's okay, I am happy to create a pull request to this repository to implement the stub generator feature!

Good idea. I think it should be included.

@MrBlenny
Copy link
Author

This is fantastic! It works wonders.

I have added a small rospypi/rosidl_generator_mypy#8 to help fix an issue I was having with some slight issues with array type annotations.

Thanks so much for your work. It would be fantastic to see this included in rosidl_python directly (and for the various common_interfaces be built directly with pyi support).

@bonprosoft
Copy link

Thank you @MrBlenny ! I'm so glad to hear that.

It would be fantastic to see this included in rosidl_python directly (and for the various common_interfaces be built directly with pyi support).

I think so, too!
I would appreciate if I could get any feedback from maintainer.


Also, I noticed that py.typed is missing in rclpy, which is problematic for mypy to work with the type annotation of rclpy.

ros2/rclpy#936

It would be wonderful if ROS2 packages work nicely with mypy.

@ros-discourse
Copy link

This issue has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/python-type-checking-in-ros2/28507/1

@InvincibleRMC InvincibleRMC linked a pull request Mar 1, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
5 participants