[issue36144] Dictionary union. (PEP 584)

[Raps Uk]

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 precedence. So don't do this:

>>> c = dict(a.items() | b.items())
This example demonstrates what happens when values are unhashable:

>>> x = {'a': []}
>>> y = {'b': []}
>>> dict(x.items() | y.items())
Traceback (most recent call last):
  File "", line 1, in 
TypeError: unhashable type: 'list'
Here's an example where y should have precedence, but instead the value from x 
is retained due to the arbitrary order of sets:

>>> x = {'a': 2}
>>> y = {'a': 1}
>>> dict(x.items() | y.items())
{'a': 2}

http://net-informations.com/python/ds/merge.htm

--
nosy: +Raps Uk

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

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 recommend just subclassing dict and overriding the operator, as the 
PEP suggests.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Sumit Jaiswal]

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, 3]}
>>> d2 = {'spam': [2, 3, 4]}
>>> d1 | d2
>>> {'spam': [1, 2, 3, 4]}

--
nosy: +justjais

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Guido van Rossum]

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 
https://github.com/python/cpython/pull/18832#issuecomment-596910350).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Curtis Bucher]

Change by Curtis Bucher :


--
pull_requests: +18583
pull_request: https://github.com/python/cpython/pull/19221

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Guido van Rossum]

Guido van Rossum  added the comment:

I'm guessing this can be closed?

--
stage: resolved -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Guido van Rossum]

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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Curtis Bucher]

Change by Curtis Bucher :


--
pull_requests: +18488
pull_request: https://github.com/python/cpython/pull/19127

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Guido van Rossum]

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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Guido van Rossum]

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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[STINNER Victor]

Change by STINNER Victor :


--
nosy:  -vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Curtis Bucher]

Change by Curtis Bucher :


--
pull_requests: +18467
pull_request: https://github.com/python/cpython/pull/19106

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Guido van Rossum]

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 that these classes' essential behavior is preserved.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

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 MutableMapping 
subclass, (not a dict subclass), we could in theory hold off on updating this 
until someone asks for it, without backward compatibility issues.

I think the other two should be updated, though. I can coordinate PRs for them 
this week.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Guido van Rossum]

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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Guido van Rossum]

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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

Change by Brandt Bucher :


--
pull_requests: +18315
pull_request: https://github.com/python/cpython/pull/18967

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

Change by Brandt Bucher :


--
pull_requests: +18283
pull_request: https://github.com/python/cpython/pull/18931

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[STINNER Victor]

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 are updated as well.

--
nosy: +vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Charles Burkland]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Curtis Bucher]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Guido van Rossum]

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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

Change by Brandt Bucher :


--
pull_requests: +18172
pull_request: https://github.com/python/cpython/pull/18814

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

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 (unless there's 
somebody who is already familiar with it who wants to take over). It looks 
well-commented, though.

I think we can pass on the http.cookies subclasses since there don't appear to 
be any experts/maintainers for that module.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Guido van Rossum]

Guido van Rossum  added the comment:

Still waiting for ChainMap -- what else?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Guido van Rossum]

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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

Change by Brandt Bucher :


--
pull_requests: +18086
pull_request: https://github.com/python/cpython/pull/18729

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

Brandt Bucher  added the comment:

Sounds good, I'll have these up soon.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Guido van Rossum]

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 and put up a PR.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

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 agreement that the 
update behavior is reasonable, even for ChainMaps.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Serhiy Storchaka]

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):
return self.__class__(*self.maps, other)


There are problems with both variants, so I think it may be better to not add 
this operator to ChainMap.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

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 check the type of the argument (for 
either dict or ChainMap, or maybe just Mapping), to keep it from working on an 
iterable of key-value pairs.

> I guess we need a __ror__ as well, in case there's some other mapping that 
> doesn't implement __or__:

Agreed. Again, we can check for Mapping here to assure success for the copy() 
move.

> As a refinement, __or__ and __ror__ should perhaps check whether the 
> operation can possibly succeed and return NotImplemented instead of raising? 
> (Based on the type of other only, not its contents.)

Yup, see above. I think a check for Mapping should be fine.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Guido van Rossum]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Guido van Rossum]

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 
would work?

def __or__(self, other):
new = self.copy()
new |= other  # OR new.update(other) ???
return new

def __ior__(self, other):
self.update(other)
return self

Note that there is no ChainMap.update() definition -- it relies on 
MutableMapping.update().

I guess we need a __ror__ as well, in case there's some other mapping that 
doesn't implement __or__:

def __ror__(self, other):
new = other.copy()
new.update(self)
return new

Note that this doesn't return a ChainMap but an instance of type(other).  If 
other doesn't have a copy() method it'll fail.

As a refinement, __or__ and __ror__ should perhaps check whether the operation 
can possibly succeed and return NotImplemented instead of raising? (Based on 
the type of other only, not its contents.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

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 tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Guido van Rossum]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

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 in-place variant. :)

>>> from collections import ChainMap
>>> d = {}
>>> c = ChainMap({"r": 2, "d":2})
>>> d |= c
>>> d
{'r': 2, 'd': 2}

I think it would be confusing to have `ChainMap | ChainMap` behave subtly 
different than `dict | ChainMap`. It would be *especially* odd if it also 
differed subtly from `ChainMap | dict`.

To recap:

+1 on adding the operators with dict semantics,
+0 on no PEP 584 for ChainMap.
-0 on implementing them, but changing the winning behavior by concatenating the 
maps lists or something. This would probably make more sense to me as a `+` 
operator, honestly. :(
-1 for having the operators behave differently (other than performance 
shortcuts) for `cm | d`, `cm | cm`, `cm |= d`, `cm |= cm`.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Guido van Rossum]

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 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.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

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 them should change, and how?

I don’t like this. Let me try to explain why:

So far (and to the best of my knowledge), setting and updating values on a 
ChainMap works exactly the same as it does for dict, with all of the same 
semantics (the docs themselves even say that “all of the usual dictionary 
methods are supported”… which now could be interpreted as meaning | and |= as 
well). It’s only when deleting or using the new interfaces that things get more 
specialized.

But that doesn’t really apply here. Having different (or worse, inconsistent) 
behavior for these operators, I feel, would be more confusing than helpful. 
Remember, a major goal of this proposal is to aid in duck typing.

So, Josh’s understanding of my intended semantics is correct, I propose that, 
for:

d1 = {'a': 1, 'b': 2}
d2 = {'b': 3, 'c': 4}
d3 = {'a': 5, 'd': 6}
d4 = {'d': 7, 'e': 8}
cm1 = ChainMap(d1, d2)
cm2 = ChainMap{d3, d4)

cm3 = cm1 | cm2

Gives cm3 a value of:

ChainMap(d1 | d4 | d3, d2)  # Or, equivalently: ChainMap(d1 | dict(cm2), d2)

And:

cm1 |= cm2

Is equivalent to:

d1 |= cm2

I don’t want to change which map is "first", and I think changing the winning 
behavior from that of dict will create more problems than it solves. We only 
need to look at how ChainMap handles the update method… it keeps the same exact 
behavior, rather than trying to be lazy or reversed or something.

If we *are* deciding to do something different, then I think it should have no 
relationship to PEP 584, which reasons out a carefully considered merge 
operation for dict, not ChainMap. But, it would also probably need a different 
operator, and be able to stand on its own merits.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Guido van Rossum]

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 `ChainMap(other, *cm.maps)`.

I've not used ChainMap much (though I've seen some code that uses it) so I'm 
probably not the best judge of whether this is a good feature to have.

Note that `other | cm` will just do whatever `other.__or__` does, since 
ChainMap isn't a true subclass of dict, so it will not fall back to 
`cm.__ror__`. Basically ChainMap will not get control in this case.

Other thoughts:

- Maybe `cm1 | cm2` (both ChainMaps) ought to return `ChainMap(*cm2.maps, 
*cm1.maps)`?

- These semantics make `|=` behave rather differently from `|`. Is that okay? 
If not, which of them should change, and how?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Josh Rosenberg]

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. Could you give examples of what the 
expected end result is for:

d1 = {'a': 1, 'b': 2}
d2 = {'b': 3, 'c': 4}
d3 = {'a': 5, 'd': 6}
d4 = {'d': 7, 'e': 8}
cm1 = ChainMap(d1, d2)
cm2 = ChainMap{d3, d4)

followed by either:

cm3 = cm1 | cm2

or
cm1 |= cm2

? As in, what is the precise state of the ChainMap cm3 or the mutated cm1, 
referencing d1, d2, d3 and d4 when they are still incorporated by references in 
the chain?

My impression from what you said is that the plan would be for the updated cm1 
to preserve references to d1 and d2 only, with the contents of cm2 (d3 and d4) 
effectively flattened and applied as an in-place update to d1, with an end 
result equivalent to having done:

cm1 = ChainMap(d1, d2)
d1 |= d4
d1 |= d3

(except the key ordering would actually follow d3 first, and d4 second), while 
cm3 would effectively be equivalent to having done (note ordering):

cm3 = ChainMap(d1 | d4 | d3, d2)

though again, key ordering would be based on d1, then d3, then d4, not quite 
matching the union behavior. And a reference to d2 would be preserved in the 
final result, but not any other original dict. Is that correct? If so, it seems 
like it's wasting ChainMap's key feature (lazy accumulation of maps), where:

cm1 |= cm2

could be equivalent to either:

cm1.maps += cm2.maps

though that means cm1 wins overlaps, where normal union would have cm2 win, or 
to hew closer to normal union behavior, make it equivalent to:

cm1.map[:0] = cm2.maps

prepending all of cm2's maps to have the same duplicate handling rules as 
regular dicts (right side wins) at the expense of changing which map cm1 uses 
as the target for writes and deletes. In either case it would hew to the spirit 
of ChainMap, making dict "union"-ing an essentially free operation, in exchange 
for increasing the costs of lookups that don't hit the top dict.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

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 update / copy-and-update, respectively.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Josh Rosenberg]

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 semantics best, but it feels... wrong, given ChainMap's 
normal left-to-right precedence. And top-mostness affects which dict receives 
all writes, so if  chain1 |= chain2 operates with dict-like precedence (chain2 
layers over chain1), then that also means the target of writes/deletions/etc. 
changes to what was on top in chain2.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Guido van Rossum]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Guido van Rossum]

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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36144] Dictionary union. (PEP 584)

[Brandt Bucher]

Change by Brandt Bucher :


--
title: Dictionary addition. (PEP 584) -> Dictionary union. (PEP 584)

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com