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

Fixed filter_globs for noetic #506

Merged
merged 2 commits into from
May 30, 2020
Merged

Conversation

foreignrobot
Copy link

Service calls with non empty requests (e.g. /rosapi/topics_for_type) were crashing due to filter's return type in python 3.

Service calls with non empty requests (e.g. /rosapi/topics_for_type) were crashing due to filter's return type in python 3.
if PYTHON2:
return filter(lambda x: any_match(x, globs), full_list)
else:
return [i for i in filter(lambda x: any_match(x, globs), full_list)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of having all this logic specific to Python version, the Python 3 path should work fine in Python 2 and should be the only path if that is the case.

Copy link
Author

@foreignrobot foreignrobot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed the Python 3 path works fine in Python 2. Thus, I've removed the extra logic.

Copy link
Contributor

@mvollrath mvollrath 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 👍

@mvollrath mvollrath merged commit ad61c2a into RobotWebTools:develop May 30, 2020
@hikinggrass
Copy link

@mvollrath is a noetic release with this fix planned in the near future (maybe for the next sync) or are there any other bugs you would like to get fixed before the next version bump?

@mvollrath
Copy link
Contributor

I don't see any reason not to tag a release, will do it soon after the new CI stuff is settled.

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.

None yet

3 participants