Skip to content

/vyper

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

Add support for Python3.8 #1678

Merged
merged 7 commits into from Nov 11, 2019
Merged

Add support for Python3.8 #1678

merged 7 commits into from Nov 11, 2019

Conversation

@iamdefinitelyahuman
Copy link
Contributor

iamdefinitelyahuman commented Nov 4, 2019

What I did

Add support for Python 3.8

How I did it

  • Replace Constant python AST type with deprecated type names when generating Vyper AST nodes.
  • Add python3.8 build to CI

How to verify it

Run the tests

Description for the changelog

  • support for Python3.8

Cute Animal Picture

image
@iamdefinitelyahuman
replace ast.Constant with deprecated type names, allows python3.8 sup…
78da2e6 
…port
@iamdefinitelyahuman
add python3.8 to tox, circleCI
Loading status checks…
84f000f
@iamdefinitelyahuman
refactor ast generation
Loading status checks…
e78a1ad
@iamdefinitelyahuman iamdefinitelyahuman force-pushed the iamdefinitelyahuman:py38-ast branch from ce61beb to e78a1ad Nov 4, 2019
@fubuloubu

This comment has been minimized.

Sign in to view
Copy link
Member

fubuloubu commented Nov 5, 2019

Huh, fuzzing actually caught something this time lol
@iamdefinitelyahuman

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

iamdefinitelyahuman commented Nov 5, 2019

Failing test should be fixed by #1679
@iamdefinitelyahuman iamdefinitelyahuman marked this pull request as ready for review Nov 5, 2019
@fubuloubu

This comment has been minimized.

Sign in to view
Copy link
Member

fubuloubu commented Nov 5, 2019

@iamdefinitelyahuman rebase master
@fubuloubu

This comment has been minimized.

Sign in to view
Copy link
Member

fubuloubu commented Nov 6, 2019

I'd like to merge #1657 first and rebase on that to ensure we're playing with all the latest toys
@iamdefinitelyahuman
Merge branch 'master' into py38-ast
Loading status checks…
aab7e5f
@iamdefinitelyahuman iamdefinitelyahuman force-pushed the iamdefinitelyahuman:py38-ast branch from 7d44c61 to aab7e5f Nov 7, 2019
@jacqueswww
jacqueswww reviewed Nov 7, 2019
View changes
vyper/ast_utils.py Outdated
else:
class_name = node.__class__.__name__

This comment has been minimized.

Sign in to view
Copy link
@jacqueswww

jacqueswww Nov 7, 2019

Collaborator

Not sure I understand the fix here, as far as I can tell all these types are replaced by Constant(value=) now, doesn't that mean we have to translate each constant type and port over the value attribute (e.g. .s for Str and .n for number) ?

This comment has been minimized.

Sign in to view
Copy link
@jacqueswww

jacqueswww Nov 7, 2019 •
edited

Collaborator

And would that mean something the replacement step would have to map something like isinstance(node.value, str) => vyper_ast.Str(s=node.value) (if isinstance(node, python_ast.Constant).

This comment has been minimized.

Sign in to view
Copy link
@iamdefinitelyahuman

iamdefinitelyahuman Nov 7, 2019

Author Contributor

See new commits based on our discussion 👍
@iamdefinitelyahuman
call isinstance on node.value to avoid future deprecation issues
cee9576
@fubuloubu

This comment has been minimized.

Sign in to view
Copy link
Member

fubuloubu commented Nov 7, 2019 •
edited

technically, eth-tester (and py-evm) doesn't support 3.8 yet. That could be why this is an issue.

cc @carver
@iamdefinitelyahuman iamdefinitelyahuman force-pushed the iamdefinitelyahuman:py38-ast branch 4 times, most recently from 24d5e4f to cbefaa8 Nov 7, 2019
@carver

This comment has been minimized.

Sign in to view
Copy link

carver commented Nov 7, 2019

technically, eth-tester (and py-evm) doesn't support 3.8 yet. That could be why this is an issue.

cc @carver

Ahh, it looks like it's the blake2b-py build that's not compatible somehow. cc @davesque
@davesque

This comment has been minimized.

Sign in to view
Copy link
Contributor

davesque commented Nov 8, 2019

Spending some time on this today.
@davesque

This comment has been minimized.

Sign in to view
Copy link
Contributor

davesque commented Nov 8, 2019

Just released blake2b-py v0.1.3 that includes support for python 3.8.
@carver carver mentioned this pull request Nov 9, 2019
Open
@iamdefinitelyahuman
adjust elif sequence to reduce indentation
Loading status checks…
23013ac
@iamdefinitelyahuman iamdefinitelyahuman force-pushed the iamdefinitelyahuman:py38-ast branch from cbefaa8 to 23013ac Nov 9, 2019
@iamdefinitelyahuman
reorder ast.Constant type checks
Loading status checks…
1c66712
@jacqueswww jacqueswww merged commit 14aa9d6 into vyperlang:master Nov 11, 2019
4 checks passed
4 checks passed
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: py36-core Your tests passed on CircleCI!
Details
ci/circleci: py37-core Your tests passed on CircleCI!
Details
ci/circleci: py38-core Your tests passed on CircleCI!
Details
@iamdefinitelyahuman iamdefinitelyahuman deleted the iamdefinitelyahuman:py38-ast branch Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacqueswww jacqueswww

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@iamdefinitelyahuman @fubuloubu @carver @davesque @jacqueswww
You can’t perform that action at this time.