classification
Title: Return function locals() in order of creation?
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ned.deily Nosy List: dabeaz, gvanrossum, ncoghlan, ned.deily, njs, rhettinger, vstinner, yselivanov
Priority: low Keywords: patch

Created on 2018-01-28 03:16 by gvanrossum, last changed 2018-02-08 21:27 by rhettinger. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 5379 merged rhettinger, 2018-01-28 08:22
PR 5390 merged miss-islington, 2018-01-28 17:41
PR 5439 merged ncoghlan, 2018-01-30 04:16
PR 5586 rhettinger, 2018-02-08 21:27
Messages (21)
msg310905 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2018-01-28 03:16
Found in this thread: https://twitter.com/dabeaz/status/956264950430912512

Note that at the global level locals() indeed returns variables in order of definition. Ditto at class scope. Because those calls just return the actual dict. But in a function, a dict is constructed. I agree with Dave that it would be nice if the order there was reversed.
msg310907 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-01-28 03:38
What should happen for:

def f():
    if random.random() < 0.5:
        a = 1
        b = 2
    else:
        b = 1
        a = 2
    return locals()

? Right now co_varnames preserves the order that names were encountered when compiling, so it'd be easy to make 'a' come before 'b' in locals(), so the above function returns either {'a': 1, 'b': 2} or {'a': 2, 'b': 1}.

If we want to preserve the illusion that local variables are entries in the locals() dict, though, then the dict ought to be either {'a': 1, 'b': 2}, {'b': 1, 'a': 2}. Making this happen would require extra machinery for an extreme edge case so I'm guessing it's not worth it though.
msg310915 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2018-01-28 04:48
The current oddity where the names will always appear in the reverse of declaration order comes from a C level loop in the frame object's "map_to_dict" helper function that loops in reverse [1]:

    ...
    for (j = nmap; --j >= 0; ) {
        ...
    }

While I haven't tried reversing that yet, I haven't found anything to indicate that it *needs* to be a reverse iteration - it looks like Guido just wrote it that way back in 1994 [2], and everyone that touched the code since then preserved the original iteration order since they didn't have a compelling reason to change it.


[1] https://github.com/python/cpython/blob/master/Objects/frameobject.c#L794
[2] https://github.com/python/cpython/commit/1d5735e84621a7fe68d361fa0e289fa2c3310836
msg310925 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2018-01-28 06:04
Heh, that's old code! IIRC I wrote a lot of loops like that, it was a little C trick I had picked up that reduced the loop overhead (since comparing to 0 is quicker than comparing to a variable). And until 3.6 it didn't really matter. I very much doubt the loop speed matters. So go ahead and try changing it!
msg310933 - (view) Author: David Beazley (dabeaz) Date: 2018-01-28 10:34
Some context:  I noticed this while discussing (in a course) a programming trick involving instance initialization and locals() that I'd encountered in the past:

    def _init(locs):
        self = locs.pop('self')
        for name, val in locs.items():
            setattr(self, name, val)

    class Spam:
        def __init__(self, a, b, c, d):
            _init(locals())

In looking at locals(), it was coming back in reverse order of method arguments (d, c, b, a, self).   To be honest, it wasn't a critical matter, but more of an odd curiosity in light of recent dictionary ordering.

I could imagine writing a slightly more general version of _init() that didn't depend on a named 'self' argument if order was preserved:

    def _init(locs):
       items = list(locs.items())
       _, self = items[0]
       for name, val in items[1:]:
           setattr(self, name, val)

Personally, I don't think the issue Nathaniel brings up is worth worrying about because it would be such a weird edge case on something that is already an edge case.  Returning variables in "lexical order"--meaning the order in which first encountered in the source seems pretty sensible to me.
msg310972 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-01-28 17:40
New changeset a4d00012565d716db6e6abe1b8f33eaaa4de416e by Raymond Hettinger in branch 'master':
bpo-32690: Preserve order of locals() (#5379)
https://github.com/python/cpython/commit/a4d00012565d716db6e6abe1b8f33eaaa4de416e
msg310976 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-01-28 18:13
New changeset 9105879bfd7133ecbac67f3e9c0bacf6e477de5a by Raymond Hettinger (Miss Islington (bot)) in branch '3.6':
bpo-32690: Preserve order of locals() (GH-5379) (#5390)
https://github.com/python/cpython/commit/9105879bfd7133ecbac67f3e9c0bacf6e477de5a
msg310977 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-01-28 18:22
Why did this fix go to 3.6?
msg311000 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-01-28 21:30
> Why did this fix go to 3.6?

The patch is trivially small and fixes an odd an unnecessary implementation quirk.  Backporting will help avoid confusing dissimilarity between 3.6 and 3.7.  It also makes maintenance easier by keeping the versions in-sync (there are a number of precedents for this).
msg311003 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-01-28 21:35
I don't have a strong opinion on this, but I worry a bit that while the change is trivial, it still might break something (doctests? some obscure code?) in the next 3.6 point release.
msg311004 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-01-28 21:37
While I appreciate the desirability of changing the behavior, it does introduce a behavior change in a 3.6.x maintenance release.  I don't have a good feel for the probability that current users or 3-party libraries are depending on the old behavior.  Anyone want to hazard a guess?  If there is a significantly non-zero probability, perhaps this should wait for 3.7.
msg311010 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-01-28 22:00
If you guys think the old code is better in some way, feel free to revert the backport :-)

I'm -0 on undoing the backpport.  ISTM that of all of changes to 3.6, this is one of the safest and it has minor benefits of removing a small irritant and keeping the code in-sync.  IIRC, 3.6 dict order is only assured for CPython but not for the language spec as a whole, so this is arguably a CPython buglet or least a surprising and unexpected behavior.  

While my instincts say forward-is-better-than-scrambled, I don't care much either way.  The only way it affects me is that as an educator, the reversed order is a unnecessary distraction from live demos that show the locals dict.
msg311014 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-01-28 22:14
> If you guys think the old code is better in some way, feel free to revert the backport :-)

Hey, it's Guido's code, it must be good :)

My only concern is that this seems to be more of an issue of unspecified behavior rather than a bug and, as such, our general policy is that the status quo wins as far as changing behavior in a maintenance release.  I don't have a really strong feeling one way or another myself; I'm just trying to play devil's advocate on behalf of the user community.  Let's see if anyone else can make a case one way or another.
msg311017 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2018-01-28 22:21
I think consistency between the 3.6.x releases ought to win here.
msg311208 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-01-30 01:13
Can someone please revert the 3.6 change, as asked by Guido?
msg311223 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-01-30 04:04
Ned, can you do the reversion?  I'm a little git/github challenged on how to revert this one.
msg311225 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-01-30 04:10
I'll make sure it's taken care of prior to 3.6.5rc1.
msg311227 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2018-01-30 04:22
Noting the git-fu required for non-master reverts:

- check out the branch of interest and ensure it's up to date
- `git checkout -b bpo-32690-revert-3.6-backport`
- `git revert 9105879bfd7133ecbac67f3e9c0bacf6e477de5a`
- edit commit message as appropriate
- create a PR targeting the branch of interest: https://github.com/python/cpython/pull/5439

Devguide issue here: https://github.com/python/devguide/issues/319
msg311231 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2018-01-30 05:27
New changeset 05f91a42cd9575ef338a67bd04855b44f6ac20c2 by Nick Coghlan in branch '3.6':
[3.6] Revert "bpo-32690: Preserve order of locals() (GH-5379) (#5390)"
https://github.com/python/cpython/commit/05f91a42cd9575ef338a67bd04855b44f6ac20c2
msg311232 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2018-01-30 05:28
OK, this is back the way it was on the 3.6 branch now, while keeping the change on the 3.7 branch.
msg311293 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-01-30 19:37
Thank Nick.
History
Date User Action Args
2018-02-08 21:27:40rhettingersetpull_requests: + pull_request5409
2018-01-30 19:37:33rhettingersetmessages: + msg311293
2018-01-30 05:28:39ncoghlansetstatus: open -> closed
versions: - Python 3.6
messages: + msg311232

resolution: fixed
stage: patch review -> resolved
2018-01-30 05:27:44ncoghlansetmessages: + msg311231
2018-01-30 04:22:32ncoghlansetmessages: + msg311227
2018-01-30 04:16:44ncoghlansetstage: resolved -> patch review
pull_requests: + pull_request5272
2018-01-30 04:10:39ned.deilysetmessages: + msg311225
2018-01-30 04:04:42rhettingersetassignee: ncoghlan -> ned.deily
messages: + msg311223
2018-01-30 04:02:17rhettingersetmessages: - msg311214
2018-01-30 02:25:55rhettingersetmessages: + msg311214
2018-01-30 01:13:38vstinnersetstatus: closed -> open

nosy: + vstinner
messages: + msg311208

resolution: fixed -> (no value)
2018-01-28 22:21:52gvanrossumsetmessages: + msg311017
2018-01-28 22:14:32ned.deilysetmessages: + msg311014
2018-01-28 22:00:27rhettingersetmessages: + msg311010
2018-01-28 21:37:22ned.deilysetnosy: + ned.deily
messages: + msg311004
2018-01-28 21:35:01yselivanovsetmessages: + msg311003
2018-01-28 21:30:21rhettingersetmessages: + msg311000
2018-01-28 18:22:39yselivanovsetnosy: + yselivanov
messages: + msg310977
2018-01-28 18:20:59rhettingersetstatus: open -> closed
stage: patch review -> resolved
resolution: fixed
versions: + Python 3.6
2018-01-28 18:13:20rhettingersetmessages: + msg310976
2018-01-28 17:41:32miss-islingtonsetpull_requests: + pull_request5225
2018-01-28 17:40:26rhettingersetnosy: + rhettinger
messages: + msg310972
2018-01-28 10:34:44dabeazsetnosy: + dabeaz
messages: + msg310933
2018-01-28 08:22:57rhettingersetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request5220
2018-01-28 06:48:20ncoghlansetassignee: ncoghlan
2018-01-28 06:04:10gvanrossumsetmessages: + msg310925
2018-01-28 04:48:11ncoghlansetnosy: + ncoghlan
messages: + msg310915
2018-01-28 03:38:58njssetnosy: + njs
messages: + msg310907
2018-01-28 03:16:31gvanrossumcreate