-
Notifications
You must be signed in to change notification settings - Fork 44
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
Make the message __repr__ for python look nicer. #60
Conversation
@@ -334,7 +334,18 @@ if isinstance(type_, AbstractNestedType): | |||
typename = self.__class__.__module__.split('.') | |||
typename.pop() | |||
typename.append(self.__class__.__name__) | |||
@[if 'import array' in imports]@ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using this runtime condition (which magically changes the logic for all fields as soon as an array field is added) use the types stored for each slot in SLOT_TYPES
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, good point. I've done this in 33e0ff6
for s in self.__slots__: | ||
field = getattr(self, s, None) | ||
if type(field) == array.array: | ||
t = repr(field.tolist()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this operation is fairly expensive (in terms of memory usage) I would suggest to keep using repr(field)
and then process the string to remove the array(typecode, )
part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this in 079d988.
98fbce1
to
ce9a5ea
Compare
Before this patch, publishing a message with "ros2 topic pub" would print something like: publishing #5: my_msgs.msg.my_msg(axes=array('f', [0.0])) While that is OK, it is kind of ugly. This patch hides the typecode and the "array" so that the output looks like: publishing #5: my_msgs.msg.my_msg(axes=[0.0]) Which is also how it looked in Crystal. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
ce9a5ea
to
079d988
Compare
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of comments - both addressed in 8e901fa.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks okay to me. But when I publish certain messages with the master branch, I don't see the "array(" prefix, for example:
$ ros2 topic pub chatter action_msgs/msg/GoalStatusArray
publisher: beginning loop
publishing #1: action_msgs.msg.GoalStatusArray(status_list=[])
Why is it that only some messages are affected?
I guess we may only see the array prefix with arrays of basic types. |
|
* Make the message __repr__ for python look nicer. Before this patch, publishing a message with "ros2 topic pub" would print something like: publishing #5: my_msgs.msg.my_msg(axes=array('f', [0.0])) While that is OK, it is kind of ugly. This patch hides the typecode and the "array" so that the output looks like: publishing #5: my_msgs.msg.my_msg(axes=[0.0]) Which is also how it looked in Crystal. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org> * Use string processing for arrays. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org> * Use the SLOT_TYPES for printing. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org> * Flake8 fix. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org> * some update Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
* Make the message __repr__ for python look nicer. Before this patch, publishing a message with "ros2 topic pub" would print something like: publishing #5: my_msgs.msg.my_msg(axes=array('f', [0.0])) While that is OK, it is kind of ugly. This patch hides the typecode and the "array" so that the output looks like: publishing #5: my_msgs.msg.my_msg(axes=[0.0]) Which is also how it looked in Crystal. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org> * Use string processing for arrays. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org> * Use the SLOT_TYPES for printing. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org> * Flake8 fix. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org> * some update Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Before this patch, publishing a message with "ros2 topic pub"
would print something like:
publishing #5: my_msgs.msg.my_msg(axes=array('f', [0.0]))
While that is OK, it is kind of ugly. This patch hides the
typecode and the "array" so that the output looks like:
publishing #5: my_msgs.msg.my_msg(axes=[0.0])
Which is also how it looked in Crystal.
Signed-off-by: Chris Lalancette clalancette@openrobotics.org
Note that this is currently targeting the
fix-array-publish
branch; we'll need to retarget once that one is merged. Also note that this is kind of an RFC; while it is quite ugly, it is a cosmetic issue, and touching the generated python code may be too risky for Dashing at this point. I'll run CI presently, but opinions welcome.