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

Construct the Module portion of the ast in a portable way #1552

Merged
merged 1 commit into from
May 21, 2019
Merged

Construct the Module portion of the ast in a portable way #1552

merged 1 commit into from
May 21, 2019

Conversation

asottile
Copy link
Contributor

Resolves #1551

@asottile asottile changed the title lConstruct the Moule portion of the ast in a portable way Construct the Moule portion of the ast in a portable way May 19, 2019
@davidism davidism added this to the 0.15.5 milestone May 19, 2019
@davidism davidism changed the title Construct the Moule portion of the ast in a portable way Construct the Module portion of the ast in a portable way May 19, 2019
@asottile
Copy link
Contributor Author

thanks for fixing the title, github got really confused when I switched bases and I apparently can't type the same thing twice!

@davidism
Copy link
Member

I added myself to the notification list for the BPO issue, I'll wait a bit to see if anyone responds.

@davidism
Copy link
Member

davidism commented May 21, 2019

Just to check, are we sure ast.parse is better than sys.version_info?

if sys.version_info >= (3, 8):
    module = ast.Module([func_code], [])
else:
    module = ast.Module([func_code])

It's at least clear what the compat fix is in this case, even though it's yet another version flag.

@asottile
Copy link
Contributor Author

when this inevitably changes again, ast.parse() will hand us a blank module object that already has all the new attributes

@davidism
Copy link
Member

Moving every closer to just generating a string of Python code 😂

@asottile
Copy link
Contributor Author

Moving every closer to just generating a string of Python code joy

lol but actually 🤦‍♂️

I'll rebase this one once #1554 merges 👍

@davidism
Copy link
Member

davidism commented May 21, 2019

Don't worry about rebasing, I'm taking care of it.

@davidism davidism merged commit 90a5114 into pallets:0.15.x May 21, 2019
@asottile asottile deleted the ast_module_diffs branch May 21, 2019 20:15
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants