-
Couldn't load subscription status.
- Fork 1.2k
some minor changes to make the build run with python 3 #4030
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
Conversation
|
@DaanHoogland in my tests compile and run works fine, but I'm having this problem with the marvin deploy script: |
|
This will need refactor in many places, maybe considering this library? https://docs.python.org/2/library/2to3.html |
|
yes @nvazquez this only addresses the build. There is another PR out for python3 changes (#3730 by @GabrielBrascher) for the system vm. I don't want to merge, as this is kind of independent. I did hit that deploy script as well, we can put that in a separate PR. or here as well, i just want to move this quickly and smaller chunks move faster ;) |
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.
Thanks for the PR @DaanHoogland. Always looking forward for PRs porting Python2 to Python3 :-)
| def xml_for(command): | ||
| name = command['name'] | ||
| async = command['async'] and ' (A)' or '' | ||
| asyncmethod = command['async'] and ' (A)' or '' |
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.
@DaanHoogland shouldn't the header of this python file be changed from #!/cygdrive/c/Python27 to #!/cygdrive/c/Python3?
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.
Sorry if this comment is redundant, I see that @nvazquez mentioned about changing other pieces as well.
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 think the cygwin specific python is not a good idea in any case #! /bin/env python would be better.
I think this is being taken into account in ther PRs so maybe i should close, @nvazquez @GabrielBrascher ?
|
The changes here are in #4479 as well, will probably close this one. |
Description
print as method
async keyword use in variable name fixed
urllib openurl from urllib2 to urllib.request
This PR covers partially issue #3195.
Types of changes
Screenshots (if appropriate):
How Has This Been Tested?