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

Fine-grained: Support Python 3 unpacking expressions (and fix crash) #4966

Merged
merged 4 commits into from
Apr 26, 2018

Conversation

JukkaL
Copy link
Collaborator

@JukkaL JukkaL commented Apr 25, 2018

Also fix crash on incompatible dictionary unpack. Fixes #4959.

Work towards #4951.

Also fix crash on incompatible dictionary unpack. Fixes #4959.

Work towards #4951.
Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

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

Two questions:

  • Why do you skip cache in all tests? It would be good to add a comment about this in tests.
  • There is a TODO item in deps.py after process_lvalue (presumably left by you) that says # TODO: star lvalue. Isn't it something that should be addressed in this PR?

@JukkaL
Copy link
Collaborator Author

JukkaL commented Apr 25, 2018

Why do you skip cache in all tests? It would be good to add a comment about this in tests.

I'm skipping because many of the cache tests aren't very valuable but they'll slow down tests. I'll add a comment about this somewhere.

There is a TODO item in deps.py after process_lvalue (presumably left by you) that says # TODO: star lvalue. Isn't it something that should be addressed in this PR?

I'll have a look.

@JukkaL JukkaL changed the title Fine-grained: Add tests for unpacking expressions (and fix crash) Fine-grained: Support Python 3 unpacking expressions (and fix crash) Apr 26, 2018
Copy link
Collaborator

@msullivan msullivan left a comment

Choose a reason for hiding this comment

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

LGTM

@JukkaL JukkaL merged commit 7b257c8 into master Apr 26, 2018
@msullivan msullivan deleted the fg-unpacking branch April 26, 2018 17:56
@asottile
Copy link
Contributor

I suspect this also fixes #3825

@emmatyping
Copy link
Collaborator

@asottile, no I just tested and that also fails on master.

@asottile
Copy link
Contributor

indeed it does, I should try things before pressing [comment] 🙃

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.

Crash on incompatible dictionary unpack
5 participants