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

Fix evaluation order of keys/values in dict comprehensions #73838

Closed
DimitrisJim mannequin opened this issue Feb 25, 2017 · 5 comments
Closed

Fix evaluation order of keys/values in dict comprehensions #73838

DimitrisJim mannequin opened this issue Feb 25, 2017 · 5 comments
Labels
3.8 interpreter-core Interpreter core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@DimitrisJim
Copy link
Mannequin

DimitrisJim mannequin commented Feb 25, 2017

BPO 29652
Nosy @rhettinger, @ncoghlan, @Janzert, @zhangyangyu, @DimitrisJim

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-09-17.23:36:16.137>
created_at = <Date 2017-02-25.19:54:43.014>
labels = ['interpreter-core', 'type-bug', '3.8']
title = 'Fix evaluation order of keys/values in dict comprehensions'
updated_at = <Date 2019-09-17.23:36:16.137>
user = 'https://github.com/DimitrisJim'

bugs.python.org fields:

activity = <Date 2019-09-17.23:36:16.137>
actor = 'brett.cannon'
assignee = 'none'
closed = True
closed_date = <Date 2019-09-17.23:36:16.137>
closer = 'brett.cannon'
components = ['Interpreter Core']
creation = <Date 2017-02-25.19:54:43.014>
creator = 'Jim Fasarakis-Hilliard'
dependencies = []
files = []
hgrepos = []
issue_num = 29652
keywords = []
message_count = 5.0
messages = ['288578', '288623', '315832', '315874', '351088']
nosy_count = 6.0
nosy_names = ['rhettinger', 'ncoghlan', 'janzert', 'xiang.zhang', 'Jim Fasarakis-Hilliard', 'EvKounis']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue29652'
versions = ['Python 3.8']

@DimitrisJim
Copy link
Mannequin Author

DimitrisJim mannequin commented Feb 25, 2017

Reported from [1] and similar to bpo-11205

Currently the evaluation order for keys and values in a dictionary comprehension follows that of assignments. The values get evaluated first and then the keys:

    def printer(v):
        print(v, end=' ')
        return v

    d = {printer(i): printer(j) for i, j in [(1, 2), (3, 4)]}
    # 2 1 4 3

This seems to conflict with the semantics as described in the Semantics section of PEP-274 [2] and according to my interpretation of the reference manual (I'd expect the evaluation to be similar to dict-displays).

How should this be addressed? Fix the evaluation order or specify this edge case an "Implementation detail" in the reference manual?

I already have a fix for this lying around (changes to compiler_sync_comprehension_generator, compiler_sync_comprehension_generator and a switch in MAP_ADD) and can make a pull request if required.

I'm not sure if this is classified as a bug per-se so I only tagged Py3.7 for it.

[1] http://stackoverflow.com/questions/42201932/order-of-operations-in-a-dictionary-comprehension
[2] https://www.python.org/dev/peps/pep-0274/#semantics

@DimitrisJim DimitrisJim mannequin added 3.7 interpreter-core Interpreter core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Feb 25, 2017
@rhettinger
Copy link
Contributor

rhettinger commented Feb 27, 2017

I think the current behavior is correct and desirable (as you say, it follows the order that would take place in an assignment, making it easy to roll-up existing for-loop code into a dict comprehension or to unroll and existing comprehension). Also, I think changing the behavior might risk introducing bugs into existing code that may have unconsciously relied on the existing order. My recommendation is to document the current value-first behavior.

For the other issue, 11205, I agree with the discussion there that key-first-value-second makes more sense in the context of literals which are normally evaluated left-to-right.

@ncoghlan
Copy link
Contributor

ncoghlan commented Apr 27, 2018

The current discrepancy is odd when you compare it to the equivalent generator expression:

{k:v for k, v in iterable}

dict(((k, v) for k, v in iterable))

It would never have occurred to me to expect the evaluation order to match a fully unrolled loop with a nested "d[k] = v" assignment, because the dict constructor doesn't work that way - it accepts an iterable of 2-tuples.

PEP-274 also specifies the iterable-of-2-tuples interpretation (using a list comprehension as its baseline rather than a generator expression): https://www.python.org/dev/peps/pep-0274/#semantics

@ncoghlan ncoghlan added 3.8 and removed 3.7 labels Apr 27, 2018
@Janzert
Copy link
Mannequin

Janzert mannequin commented Apr 29, 2018

Just as a note so the email discussion isn't forever lost to the void.

In an unrelated thread on python-dev recently there was a short discussion on this topic in which both Guido van Rossum[1] and Tim Peters[2] gave the opinion that this should change should probably be made.

1: https://mail.python.org/pipermail/python-dev/2018-April/153122.html
2: https://mail.python.org/pipermail/python-dev/2018-April/153134.html

@DimitrisJim
Copy link
Mannequin Author

DimitrisJim mannequin commented Sep 3, 2019

This was indeed changed as part of PEP-572. Should be closed as fixed.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.8 interpreter-core Interpreter core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants