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

[noetic] Fix install and improve Python 3 support #94

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

mikaelarguedas
Copy link

@mikaelarguedas mikaelarguedas commented Dec 1, 2020

This PR allow the capability server to start, currently it fails with:

/usr/bin/env: ‘python’: No such file or directory

The third commit runs modernize on the codebase which fixed all the Python3 problems I was facing when trying to run the demo

Verified

This commit was created on github.com and signed with GitHub’s verified signature.
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
@mikaelarguedas
Copy link
Author

mikaelarguedas commented Dec 1, 2020

Remaining test failures are due to the package pycodestyle having a faulty rosdep rule (ros/rosdistro#27546) and is unrelated to this change

@mikaelarguedas
Copy link
Author

@ros-pull-request-builder retest this please

@stwirth
Copy link

stwirth commented Jan 28, 2021

👍

@mikaelarguedas
Copy link
Author

@cottsay @sloretz how does that PR following your work of porting to python3 look to you?
Do you think we could get this merged and released in Noetic ?

@Rezenders
Copy link

Rezenders commented Apr 29, 2021

Just pinging this thread. Is it possible to merge it?
I tested @mikaelarguedas modifications with python3 and it worked.

@wjwwood @cottsay @sloretz

@mikaelarguedas
Copy link
Author

agreed, modernize tends to introduce unnecessary changes.
As there is interest in this PR, happy to take a stab at reducing the number of unnecessary changes added by it.
I do not have the setup I used it to test it last anymore though

Your Name and others added 3 commits April 29, 2021 20:05
@mikaelarguedas mikaelarguedas force-pushed the fix_install_and_python branch from 4785e13 to 7de19ba Compare April 29, 2021 20:22
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
@mikaelarguedas mikaelarguedas force-pushed the fix_install_and_python branch from b8d28e1 to 3ff4c8f Compare April 29, 2021 20:53
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
@mikaelarguedas
Copy link
Author

@wjwwood Here is a shorter version of this PR, PTAL.
I removed the list that seemed unnecessary, ended up adding back a couple in the places where elements from the dicts were being deleted within the loop.
Also took advantage to use YAML's Safe Loader to silence some warning.

@stwirth @Rezenders could you check if this version works for you ?

@wjwwood
Copy link
Member

wjwwood commented May 19, 2021

I'm not sure what the source of the error from travis-ci is, but it would be best to fix that.

The melodic failure looks related:

TypeError: unicode argument expected, got 'str'

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

4 participants