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

feature/465 drop support for python27 #466

Merged

Conversation

cryptomental
Copy link
Contributor

@cryptomental cryptomental commented Jun 22, 2018

What was wrong?

Python 2 Support ends. Additionally some dependencies no longer work for Python 2.7.

How was it fixed?

Use 2to3 tool to apply a set of fixers to convert code into Python3 specific syntax, remove Python 2.7 from tox, Travis and setup.py

Cute Animal Picture

@cryptomental cryptomental changed the title Feature/465 drop support for python27 WIP Feature/465 drop support for python27 Jun 22, 2018
@cryptomental
Copy link
Contributor Author

Two tests failed, likely caused by 2to3 refactoring. WIP.

@voith
Copy link
Collaborator

voith commented Jun 22, 2018

@cryptomental This was on my TODO list. Thank you for taking this up. If you need any help let me know!

2to3 tool applies a series of fixers to transform code into Python 3.x specific
syntax. Used command:

2to3 -x dict -n -w populus/

Refs: ethereum#465
@cryptomental cryptomental force-pushed the feature/465-drop-support-for-python27 branch from 2358ced to c3392fa Compare June 22, 2018 14:18
Remove implementation and conditional import in __init__.

Refs: ethereum#465
2to3 tool incorrectly changed __nonzero__ to __bool__,
leaving two methods with the same name.

Refs: ethereum#465
@cryptomental
Copy link
Contributor Author

Thanks @voith ! Build is green and so will be master after the merge. I fixed the error introduced by 2to3 tool and also removed Python 2 version of six util.

@cryptomental cryptomental changed the title WIP Feature/465 drop support for python27 feature/465 drop support for python27 Jun 22, 2018
@@ -9,7 +9,7 @@


if sys.version_info.major == 2:
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this whole if block shouldn't be necessary anymore.

@voith
Copy link
Collaborator

voith commented Jun 26, 2018

@cryptomental Can you make the small change suggested by carver?

@voith voith mentioned this pull request Jun 27, 2018
@@ -12,7 +12,7 @@ class VyperBackend(BaseCompilerBackend):

def get_compiled_contracts(self, source_file_paths, import_remappings):
try:
from vyper import compiler
from .vyper import compiler
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seemed wrong to me and I've fixed this in #469. Although vyper library and this module have the same name, import inside the function works fine.

@voith
Copy link
Collaborator

voith commented Jun 27, 2018

@cryptomental I created a new PR #469 with your changes cherry-picked. I hope you don't mind! I have added the change that @carver suggested and also fixed a bug that was introduced in this PR. I also added the new style super calls in python3.

@carver carver merged commit 5a01c58 into ethereum:master Jun 27, 2018
@cryptomental
Copy link
Contributor Author

@voith , @carver thank you for merging this. The notification drowned in my recently flooded mailbox and I discovered it now.

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