-
Notifications
You must be signed in to change notification settings - Fork 111
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
handle infinity in python generation, fixes #71 #77
Conversation
@ros/ros_team Can you have a look at this when you have time? It's working as expected but I'm wondering if this could be cleaner. Thanks! |
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.
lgtm, minus a small comment.
elif isinstance(v, list): | ||
for i in v: | ||
if isinstance(i, str) and i == old_str: | ||
i = new_val |
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.
It would be clearer (in my opinion) if you assigned it directly back into orig_dict
as you do in line 590.
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.
done in 53ad18e, thanks for the feedback
@@ -581,11 +581,13 @@ def _rreplace_str_with_val_in_dict(self, orig_dict, old_str, new_val): | |||
if isinstance(v, dict): | |||
self._rreplace_str_with_val_in_dict(v, old_str, new_val) | |||
elif isinstance(v, list): | |||
idx = 0 |
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.
You can use enumerate()
here, to save yourself a line. It would look like this:
for idx, i in enumerate(v):
if isinstance(i, str) and i == old_str:
orig_dict[k][idx] = new_val
elif isinstance(i, dict):
self._rreplace_str_with_val_in_dict(i, old_str, new_val)
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.
good point, forgot that was a thing. Thanks! 03468be
03468be
to
5104be4
Compare
This replaces every occurrence of
std::numeric_limits<double>::infinity()
in the configuration dictionary before dumping it in the python generated file. Fixes #71.