[issue36144] Dictionary union. (PEP 584)

Raps Uk added the comment: Taking the union of items() in Python 3 (viewitems() in Python 2.7) will also fail when values are unhashable objects (like lists, for example). Even if your values are hashable, since sets are semantically unordered, the behavior is undefined in regards to

[issue36144] Dictionary union. (PEP 584)

Brandt Bucher added the comment: Similar behavior was considered and ultimately rejected by the PEP as being too specialized: https://www.python.org/dev/peps/pep-0584/#concatenate-values-in-a-list What you're asking for it subtly different, and even *more* specialized than that. I'd

[issue36144] Dictionary union. (PEP 584)

Sumit Jaiswal added the comment: First off, thanks for adding the feature, it's much appreciated. But it'd be great if you guys can enable list merge for the dict having list as its value, in current form I believe it's handling only "key: value" merge. for e.g.: >>> d1 = {'spam': [1, 2,

[issue36144] Dictionary union. (PEP 584)

Guido van Rossum added the comment: I guess we should keep this open until Raymond Hettinger has given feedback on https://github.com/python/cpython/pull/18832 (where we have the option of changing to Brandt's proposal from

[issue36144] Dictionary union. (PEP 584)

Change by Curtis Bucher : -- pull_requests: +18583 pull_request: https://github.com/python/cpython/pull/19221 ___ Python tracker ___

[issue36144] Dictionary union. (PEP 584)

Guido van Rossum added the comment: I'm guessing this can be closed? -- stage: resolved -> patch review ___ Python tracker ___ ___

[issue36144] Dictionary union. (PEP 584)

Brandt Bucher added the comment: And... that's it! Big thanks to everybody who had a part in making this happen. -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker

[issue36144] Dictionary union. (PEP 584)

Guido van Rossum added the comment: New changeset 8f1ed21ecf57cc8b8095d9d1058af2b9b3ed0413 by Curtis Bucher in branch 'master': bpo-36144: Add union operators to WeakValueDictionary584 (#19127) https://github.com/python/cpython/commit/8f1ed21ecf57cc8b8095d9d1058af2b9b3ed0413 --

[issue36144] Dictionary union. (PEP 584)

Change by Curtis Bucher : -- pull_requests: +18488 pull_request: https://github.com/python/cpython/pull/19127 ___ Python tracker ___

[issue36144] Dictionary union. (PEP 584)

Guido van Rossum added the comment: New changeset 25e580a73c163f472fdeb5489bebef85da21655c by Curtis Bucher in branch 'master': bpo-36144: Add union operators to WeakKeyDictionary (#19106) https://github.com/python/cpython/commit/25e580a73c163f472fdeb5489bebef85da21655c --

[issue36144] Dictionary union. (PEP 584)

Guido van Rossum added the comment: New changeset f393b2c588559162dc2e77f8079a42e48558870a by Curtis Bucher in branch 'master': bpo-36144: Add PEP 584 operators to collections.ChainMap (#18832) https://github.com/python/cpython/commit/f393b2c588559162dc2e77f8079a42e48558870a --

[issue36144] Dictionary union. (PEP 584)

Change by STINNER Victor : -- nosy: -vstinner ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue36144] Dictionary union. (PEP 584)

Change by Curtis Bucher : -- pull_requests: +18467 pull_request: https://github.com/python/cpython/pull/19106 ___ Python tracker ___

[issue36144] Dictionary union. (PEP 584)

Guido van Rossum added the comment: I definitely think we should leave Shelf alone, it's a toy class from a different era. It makes sense to update the weak dicts; hopefully the | and |= operators can be implemented in terms of other, more primitive operations, so we will have assurance

[issue36144] Dictionary union. (PEP 584)

Brandt Bucher added the comment: Three other MutableMappings we might want to update: - shelve.Shelf - weakref.WeakKeyDictionary - weakref.WeakValueDictionary Shelf is up in the air, since it doesn't look like it defines a copy() equivalent... I also have no experience with it. Since it's a

[issue36144] Dictionary union. (PEP 584)

Guido van Rossum added the comment: New changeset 6d674a1bf456945eb758e85c11484a9f1494f2b4 by Brandt Bucher in branch 'master': bpo-36144: OrderedDict Union (PEP 584) (#18967) https://github.com/python/cpython/commit/6d674a1bf456945eb758e85c11484a9f1494f2b4 --

[issue36144] Dictionary union. (PEP 584)

Guido van Rossum added the comment: New changeset d648ef10c5c7659ed3c9f34d5c751dc55e2c6007 by Charles Burkland in branch 'master': bpo-36144: Update os.environ and os.environb for PEP 584 (#18911) https://github.com/python/cpython/commit/d648ef10c5c7659ed3c9f34d5c751dc55e2c6007 --

[issue36144] Dictionary union. (PEP 584)

Change by Brandt Bucher : -- pull_requests: +18315 pull_request: https://github.com/python/cpython/pull/18967 ___ Python tracker ___

[issue36144] Dictionary union. (PEP 584)

Brandt Bucher added the comment: Yes, I can add a section explaining that after the PEP was accepted, we decided to add the operators to several non-dict mappings as well. I'm also going to add some explanation as to why Mapping/MutableMapping didn't grow them, too. --

[issue36144] Dictionary union. (PEP 584)

Change by Brandt Bucher : -- pull_requests: +18283 pull_request: https://github.com/python/cpython/pull/18931 ___ Python tracker ___

[issue36144] Dictionary union. (PEP 584)

STINNER Victor added the comment: Once this issue will be done, would you mind to update PEP 584? Currently, it only says "This PEP proposes adding merge (|) and update (|=) operators to the built-in dict class." It doesn't mention that other types like OrderedDict, MappingProxy or ChainMap

[issue36144] Dictionary union. (PEP 584)

Change by Charles Burkland : -- nosy: +chaburkland nosy_count: 12.0 -> 13.0 pull_requests: +18266 pull_request: https://github.com/python/cpython/pull/18911 ___ Python tracker

[issue36144] Dictionary union. (PEP 584)

Brandt Bucher added the comment: Issue 39857 just reminded me that we should update os._Environ as well (the type of os.environ and os.environb). I have another first-timer who will probably want to take it. -- ___ Python tracker

[issue36144] Dictionary union. (PEP 584)

Change by Curtis Bucher : -- nosy: +curtisbucher nosy_count: 11.0 -> 12.0 pull_requests: +18189 pull_request: https://github.com/python/cpython/pull/18832 ___ Python tracker

[issue36144] Dictionary union. (PEP 584)

Guido van Rossum added the comment: New changeset 4663f66f3554dd8e2ec130e40f6abb3c6a514775 by Brandt Bucher in branch 'master': bpo-36144: Update MappingProxyType with PEP 584's operators (#18814) https://github.com/python/cpython/commit/4663f66f3554dd8e2ec130e40f6abb3c6a514775 --

[issue36144] Dictionary union. (PEP 584)

Change by Brandt Bucher : -- pull_requests: +18172 pull_request: https://github.com/python/cpython/pull/18814 ___ Python tracker ___

[issue36144] Dictionary union. (PEP 584)

Brandt Bucher added the comment: My brother will have a ChainMap PR up soon. I'm just finishing up MappingProxyType, myself. Probably both this weekend. Then I'll move on to OrderedDict, which looks like it could be tricky. I'll need to familiarize myself with the implementation better

[issue36144] Dictionary union. (PEP 584)

Guido van Rossum added the comment: Still waiting for ChainMap -- what else? -- ___ Python tracker ___ ___ Python-bugs-list

[issue36144] Dictionary union. (PEP 584)

Guido van Rossum added the comment: New changeset 57c9d1725689dde068a7fccaa7500772ecd16d2e by Brandt Bucher in branch 'master': bpo-36144: Implement defaultdict union (GH-18729) https://github.com/python/cpython/commit/57c9d1725689dde068a7fccaa7500772ecd16d2e --

[issue36144] Dictionary union. (PEP 584)

Change by Brandt Bucher : -- pull_requests: +18086 pull_request: https://github.com/python/cpython/pull/18729 ___ Python tracker ___

[issue36144] Dictionary union. (PEP 584)

Brandt Bucher added the comment: Sounds good, I'll have these up soon. -- ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue36144] Dictionary union. (PEP 584)

Guido van Rossum added the comment: We already have somewhat different semantics of `|` for Counter, and hence I think it's fine to give it the most useful semantics for ChainMap given that class's special behavior. I think we've come up with the right solution there. Let's stop the debate

[issue36144] Dictionary union. (PEP 584)

Brandt Bucher added the comment: I think we're only seriously considering the first variant (although implemented slightly differently, see my last two messages). And __ror__ would probably change, returning the type of self. What are the "problems" with it, exactly? We seem to be in

[issue36144] Dictionary union. (PEP 584)

Serhiy Storchaka added the comment: 1. def __or__(self, other): return self.__class__(self.maps[0] | other, *self.maps[1:]) def __ror__(self, other): return other | dict(self) 2. def __or__(self, other): return self.__class__(other, *self.maps) def __ror__(self, other):

[issue36144] Dictionary union. (PEP 584)

Brandt Bucher added the comment: Just to clarify: If we decide to check isinstance(other, (ChainMap, dict)), '|' should probably be used. If we decide to check isinstance(other, Mapping), I think the copy/update methods should be used. -- ___

[issue36144] Dictionary union. (PEP 584)

Brandt Bucher added the comment: > Im not sure if the dict(other) cast is the best way to go about it. Maybe > this would work? Yeah, I was imagining something like that... I used the cast for brevity in my reply but that probably wasn't helpful. Note that for __or__, we probably want to

[issue36144] Dictionary union. (PEP 584)

Guido van Rossum added the comment: I didn't see your second reply, with `ChainMap(other, *cm.maps)`. I'm not so keen on that, because its special behavior can't be mimicked by `|=`. -- ___ Python tracker

[issue36144] Dictionary union. (PEP 584)

Guido van Rossum added the comment: OK, that makes sense, it works similar to ChainMap.copy(), which copies maps[0] and keeps links to the rest. So in particular `cm | {}` will do the same thing as cm.copy(). Im not sure if the dict(other) cast is the best way to go about it. Maybe this

[issue36144] Dictionary union. (PEP 584)

Brandt Bucher added the comment: ...however, I could also see the (similar): ChainMap(other, *cm.maps) # Note that `other` is the original reference here. Being okay as well. Maybe even better, now that I've written it out. -- ___ Python

[issue36144] Dictionary union. (PEP 584)

Brandt Bucher added the comment: I believe that: cm | other Should return the equivalent of: ChainMap(cm.maps[0] | dict(other), *cm.maps[1:]) -- ___ Python tracker

[issue36144] Dictionary union. (PEP 584)

Guido van Rossum added the comment: OK, assuming `|=` gets the same semantics as update(), can you repeat once more (without motivation) what the specification for `cm | other` will be? -- ___ Python tracker

[issue36144] Dictionary union. (PEP 584)

Brandt Bucher added the comment: > Note that in your last message, `d1 |= cm2` will fail for this reason. You > can of course fix that with `d1 |= dict(cm2)`, although IIUC there's no > reason one of the maps couldn't be some other [Mutable]Mapping. Mappings and iterables are fine for the

[issue36144] Dictionary union. (PEP 584)

Guido van Rossum added the comment: I had just come to a different conclusion. Maybe ChainMap should just not grow `|` and `|=` operators? That way there can be no confusion. `dict() | ChainMap()` and `ChainMap() | dict()` will fail because ChainMap doesn't inherit from dict. (Note that in

[issue36144] Dictionary union. (PEP 584)

Brandt Bucher added the comment: > I think for `|=` the only choice is for it to be essentially an alias to > `.update()`. So that means `cm |= other` becomes `cm.maps[0].update(other)`. Agreed. > These semantics make `|=` behave rather differently from `|`. Is that okay? > If not, which of

[issue36144] Dictionary union. (PEP 584)

Guido van Rossum added the comment: I think for `|=` the only choice is for it to be essentially an alias to `.update()`. So that means `cm |= other` becomes `cm.maps[0].update(other)`. For `|` we are breaking new ground and we could indeed make `cm | other` do something like

[issue36144] Dictionary union. (PEP 584)

Josh Rosenberg added the comment: Sorry, I think I need examples to grok this in the general case. ChainMap unioned with dict makes sense to me (it's equivalent to update or copy-and-update on the top level dict in the ChainMap). But ChainMap unioned with another ChainMap is less clear.

[issue36144] Dictionary union. (PEP 584)

Brandt Bucher added the comment: The plan is to follow dict’s semantics. The |= operator will basically delegate to the first map in the chain. The | operator will create a new ChainMap where the first map is the merged result of the old first map, and the others are the same. So, basically

[issue36144] Dictionary union. (PEP 584)

Josh Rosenberg added the comment: What is ChainMap going to do? Normally, the left-most argument to ChainMap is the "top level" dict, but in a regular union scenario, last value wins. Seems like layering the right hand side's dict on top of the left hand side's would match dict union

[issue36144] Dictionary union. (PEP 584)

Brandt Bucher added the comment: Yep. I'm currently working on OrderedDict, defaultdict, and MappingProxyType. My brother is looking to make his first contribution, so he'll be taking care of ChainMap. -- ___ Python tracker

[issue36144] Dictionary union. (PEP 584)

Guido van Rossum added the comment: @Brandt: you have some more followup PRs planned right? Let's keep this issue open until you've done all of those. -- ___ Python tracker

[issue36144] Dictionary union. (PEP 584)

Guido van Rossum added the comment: New changeset d0ca9bd93bb9d8d4aa9bbe939ca7fd54ac870c8f by Brandt Bucher in branch 'master': bpo-36144: Document PEP 584 (GH-18659) https://github.com/python/cpython/commit/d0ca9bd93bb9d8d4aa9bbe939ca7fd54ac870c8f --

[issue36144] Dictionary union. (PEP 584)

Change by Brandt Bucher : -- title: Dictionary addition. (PEP 584) -> Dictionary union. (PEP 584) ___ Python tracker ___ ___