ros2 / rosidl_python

rosidl support for Python
Apache License 2.0
19 stars 45 forks source link

Serialization No Longer Allows float("inf") for float32 Type #169

Closed connoranderson closed 1 year ago

connoranderson commented 2 years ago

In galactic release, we were able to use the numeric-limit value of float("inf") for a float32 type. When https://github.com/ros2/rosidl_python/pull/128 was introduced (and subsequently we began experimenting with humble release), we observed that the numeric limit checks prevented this use case. This is creating inconsistencies as our c++ code is still able to serialize using std::numeric_limits<T>::infinity but the python code is not able to do so.

Bug report

Required Info:

Steps to reproduce issue

my_msg = Msg() # info field is float32
my_msg.info = float("inf")

Expected behavior

Set my_msg.info to INF (formerly would serialize to 3.402823e+40 in galactic)

Actual behavior

AssertionError: The 'info' field must be a float in [-3.402823e+38, 3.402823e+38]
maspe36 commented 2 years ago

This is also a problem for float("nan")

sloretz commented 1 year ago

Thanks for the report! The problem with nan was reported in #162, and #167 should fix both that and this issue.

connoranderson commented 1 year ago

@sloretz sounds good. Can we make sure that PR gets a backport to humble release?

connoranderson commented 1 year ago

@sloretz is there an ETA expected for #167 to merge?

connoranderson commented 1 year ago

@sloretz do you think this will be backported to humble? Looks like there was a windows compatibility issue, as well, so just want to make sure that this fix finds its way to humble.

aditya2592 commented 1 year ago

Hi, will this fix be backported to Humble?

clalancette commented 1 year ago

Hi, will this fix be backported to Humble?

Yes, it's already done via https://github.com/ros2/rosidl_python/pull/188 , and will be in the next patch release (later this month).