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

Use generator to unpack list comprehension #444

Merged
merged 1 commit into from
May 18, 2021

Conversation

MarcoGorelli
Copy link
Contributor

Addresses part of #257

Comment on lines +18 to +23
if sys.version_info < (3, 8): # pragma: no cover (py38+)
start = i - 1
while not (tokens[start].name == 'OP' and tokens[start].src == '['):
start -= 1
else: # pragma: no cover (<py38)
start = i
Copy link
Owner

Choose a reason for hiding this comment

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

I think there might be a helper for this somewhere in pyupgrade already? -- maybe try the set / dict comp rewrites?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've had a look at _arg_token_index and victims from pyupgrade._token_helpers, but it doesn't seem so easy to adapt them here.

They cover the case when the node yielded contains a function (such as set) which contains a comprehension. Here, on the other hand, the yielded node is the comprehension itself, and in <py38 I need to go backwards to find the starting bracket (rather than forward to find the first element).

I'll take a closer look / have a harder think anyway

Copy link
Contributor Author

@MarcoGorelli MarcoGorelli May 16, 2021

Choose a reason for hiding this comment

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

Having looked at this further, I can't figure out how to use victims here...any hints? Else I'll get back to it later in the week, bit stuck on this

In all the other examples of using victims I could find in pyupgrade, when it's called, you have that e.g. tokens[i] is set or dict, so then tokens[i+1] is ( and you can call victims(tokens, i+1, ...) and find the opening and closing brackets.

Here, however, I have that in <py38, tokens[i] is the element immediately after the opening bracket, while in py38+ tokens[i] is the opening bracket itself.

Copy link
Owner

Choose a reason for hiding this comment

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

it's probably fine as is

Copy link
Owner

@asottile asottile left a comment

Choose a reason for hiding this comment

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

@asottile asottile merged commit 3ec0e23 into asottile:master May 18, 2021
@MarcoGorelli MarcoGorelli deleted the unpack-list-comp branch May 18, 2021 07:31
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.

2 participants