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

Python3 compatibility #10

Merged
merged 4 commits into from
Feb 13, 2017
Merged

Python3 compatibility #10

merged 4 commits into from
Feb 13, 2017

Conversation

smits
Copy link
Contributor

@smits smits commented Dec 29, 2016

urlgrabber is not available for python3 (at least not on OSX) so I replaced the urlgrabber usage in the python version of resource_retriever with the python built in urllib/urllib2.
In addition I replaced the home baked rospack_find by rospkg and added unit tests for the python api.

Ruben Smits and others added 4 commits December 29, 2016 15:57
As urlgrabber is not supported for Python 3 replace it with either the built-in urllib (Python 2) or urllib2 (Python 3)
@smits
Copy link
Contributor Author

smits commented Dec 29, 2016

Please take a look @jacquelinekay @mikepurvis

@mikepurvis
Copy link
Member

Looks sane to me. Might make sense to add a travis config for ensuring coverage of py2 and py3, but that's up to the OSRF maintainers.

@smits
Copy link
Contributor Author

smits commented Feb 13, 2017

@jacquelinekay Any chances on getting this in?

@mikepurvis
Copy link
Member

I think it's @isucan maintaining this now.

This should definitely merge in preparation for the Lunar release.

@DLu DLu merged commit 0124b46 into ros:kinetic-devel Feb 13, 2017
@DLu
Copy link
Contributor

DLu commented Feb 13, 2017

I wrote the Python one, so I felt comfortable merging this in.

I know that @clalancette and @sloretz were taking over some of @jacquelinekay 's stuff (ros/urdf_parser_py#12). Perhaps this is another candidate.

@clalancette
Copy link
Contributor

Yeah, we did take over some of @jacquelinekay 's stuff, but since this one seemed to be maintained by Ioan (Jackie was never listed in the package.xml), we haven't looked at it at all.

@wjwwood
Copy link
Member

wjwwood commented Feb 27, 2017

@clalancette if you look at the commits she was definitely maintaining this one too, Jackie probably just didn't update the package.xml. I'd consider this another package worth picking up. Though it doesn't necessarily mean you guys have to pick it up, it would make sense given the relationship between this package and the others.

It will be blocking the release of robot_model but also rviz, so I could probably pick it up too.

@clalancette
Copy link
Contributor

Got it. Shane and I have now taken maintainership of it.

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

5 participants