PEP 709, for the third and (hopefully) final time
carljm opened this issue · 5 comments
In light of the SC feedback that the change to class-scope name visibility in comprehensions discussed in https://discuss.python.org/t/pep-709-one-behavior-change-that-was-missed-in-the-pep/26691 is not acceptable for 3.12, @JelleZijlstra and I have been exploring options B and C as outlined in that post for keeping the optimization with the originally-approved semantics, and no additional behavior changes.
Jelle has a PR which preserves inlining of all list/dict/set comprehensions, while also preserving the previous scoping rules for names in class-scope comprehensions, so it exactly implements the originally-approved semantics of PEP 709. It turns out to be a pretty simple change after all: python/cpython#104528.
I checked with @Yhg1s as 3.12 release manager, and he is OK with moving ahead with Jelle's PR for 3.12.
But he asked me to also check here to see if anyone on the SC has a strong preference for the alternative fix, which is to disable inlining altogether in module and class scopes, where it is unlikely to be performance sensitive. I also have a PR up for that: python/cpython#104519. Relative to Jelle's PR, this approach keeps the compiler a bit simpler, but it means that the subtle behavior differences discussed in PEP 709 (regarding tracebacks and locals()) would be different for function-scoped comprehensions than for class/module-scoped ones.
If the semantics are fixed then I'm fine with the change (the beta will help tell us if it isn't enough).
Same here, I'm in favor of Jelle's PR (nice work!) and agree with Brett and Thomas.
I'm strongly in favor of the updated approach to preserve inlining of all comprehensions as well as preserve the previous scoping rules (for now, until a greater conversation around class scopes and generators can be had).
Glad there was a good solution here, thanks to both @carljm and @JelleZijlstra for the great work!
I believe @Yhg1s already told you go move forward with @JelleZijlstra's PR. So lets make that the plan. If @pablogsal who hasn't had a chance to weigh has issues with the rest of us on the steering council for this, or other surprises are revealed during the Beta, we can figure out if anything needs to be reconsidered. :)
Sorry for the late replay, I had a nightmare day and couldn't get to this until now. I am very much in favour of the change and I want to thank both @carljm and @JelleZijlstra for their hard work, understanding and patience dealing with this issue. You rock 🤘