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

bpo-36144: Dictionary Union (PEP 584) #12088

Merged
merged 39 commits into from Feb 25, 2020
Merged
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
9397506
Create dict_add and dict__iadd dict methods.
brandtbucher Feb 28, 2019
c69108a
Fix failing "sum" test.
brandtbucher Feb 28, 2019
dd42f4f
Update collections.UserDict.
brandtbucher Feb 28, 2019
f59cdab
Update test_dict.py.
brandtbucher Feb 28, 2019
d895b9b
Update ACKS.
brandtbucher Feb 28, 2019
da8e2e3
Fixed whitespace.
brandtbucher Feb 28, 2019
91d6466
Update doctest count for builtins.
brandtbucher Feb 28, 2019
b2eced1
📜🤖 Added by blurb_it.
blurb-it Feb 28, 2019
bf0b383
Fix error handling.
brandtbucher Feb 28, 2019
e660046
Be "nicer" to subclasses than other built-in types.
brandtbucher Mar 2, 2019
4c0223b
Add dict subtraction operations.
brandtbucher Mar 2, 2019
2cb1629
Add new subtraction methods to collections.UserDict.
brandtbucher Mar 2, 2019
05ed4cf
Add new dict subtraction tests.
brandtbucher Mar 2, 2019
594ae27
📜🤖 Added by blurb_it.
blurb-it Mar 2, 2019
d1c830c
Merge branch 'addiction' of https://github.com/brandtbucher/cpython i…
brandtbucher Mar 2, 2019
f7ed0a2
Better error handling and "keys" method lookup.
brandtbucher Mar 3, 2019
8be454a
Clean up comments, names, and logic.
brandtbucher Mar 6, 2019
63c36dd
Update commented code.
brandtbucher Mar 7, 2019
2ba0580
Update new collections.UserDict operators.
brandtbucher Mar 22, 2019
fa44826
Add | and |= operators as aliases to + and +=.
brandtbucher Mar 22, 2019
5d77bf8
Fix whitespace... again.
brandtbucher Mar 22, 2019
1793169
Catch up with master.
brandtbucher Oct 14, 2019
362657e
Remove sub/isub/or/ior operators.
brandtbucher Oct 14, 2019
04fb96e
Remove extra whitespace.
brandtbucher Oct 16, 2019
567a218
Use copy() instead of __new__() and update(self).
brandtbucher Oct 16, 2019
159ebda
Clarify NEWS entry.
brandtbucher Oct 15, 2019
28c5c56
+ -> |
brandtbucher Dec 28, 2019
6951380
Update UserDict.
brandtbucher Dec 28, 2019
514e1ee
Catch up with master.
brandtbucher Dec 28, 2019
3567247
Modernize UserDict.__or__ trio.
brandtbucher Feb 5, 2020
4588b56
Rename test.
brandtbucher Feb 5, 2020
977ffcd
Feedback from code review.
brandtbucher Feb 5, 2020
a26a98e
Actually call the "copy" and "update" methods of subclasses.
brandtbucher Feb 5, 2020
dfa7009
Consolidate shared logic.
brandtbucher Feb 5, 2020
7c40ba6
Check the return type of self.copy().
brandtbucher Feb 6, 2020
b977a27
Add tests for subclasses.
brandtbucher Feb 6, 2020
f09ba7a
Strip whitespace.
brandtbucher Feb 6, 2020
ec33664
Remove calls to "update" method when merging.
brandtbucher Feb 7, 2020
4afd3b6
Revert subclass-preserving stuff.
brandtbucher Feb 17, 2020
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -979,6 +979,26 @@ def __contains__(self, key):

# Now, add the methods in dicts but not in MutableMapping
def __repr__(self): return repr(self.data)

def __or__(self, other):
if isinstance(other, UserDict):
return self.__class__(self.data | other.data)
if isinstance(other, dict):
return self.__class__(self.data | other)
return NotImplemented
def __ror__(self, other):
This conversation was marked as resolved by brandtbucher

This comment has been minimized.

@serhiy-storchaka

serhiy-storchaka Feb 5, 2020
Member

Please add an empty line between methods to conform PEP 8.

This comment has been minimized.

@gvanrossum

gvanrossum Feb 5, 2020
Member

But that would not conform to how the rest of this class is written. I'd leave this be.

This comment has been minimized.

@brandtbucher

brandtbucher Feb 5, 2020
Author Member

That was my reasoning. I won't touch this, then.

This comment has been minimized.

@methane

methane Feb 7, 2020
Member

__ror__ is needed here?

This __ror__ supports only UserDict and dict. But both implements __or__.
When is this __ror__ called?

This comment has been minimized.

@brandtbucher

brandtbucher Feb 7, 2020
Author Member

@methane It is used.

The first branch is sometimes taken when the LHS is a UserDict subclass that overrides __or__ and the RHS is a UserDict. The second branch is always taken when the LHS is a dict and the RHS is a UserDict.

if isinstance(other, UserDict):
return self.__class__(other.data | self.data)
if isinstance(other, dict):
return self.__class__(other | self.data)
return NotImplemented
def __ior__(self, other):
if isinstance(other, UserDict):
self.data |= other.data
else:
self.data |= other
return self

def __copy__(self):
inst = self.__class__.__new__(self.__class__)
inst.__dict__.update(self.__dict__)
@@ -37,6 +37,38 @@ def test_literal_constructor(self):
dictliteral = '{' + ', '.join(formatted_items) + '}'
self.assertEqual(eval(dictliteral), dict(items))

def test_merge_operator(self):

a = {0: 0, 1: 1, 2: 1}
b = {1: 1, 2: 2, 3: 3}

c = a.copy()
c |= b

self.assertEqual(a | b, {0: 0, 1: 1, 2: 2, 3: 3})
This conversation was marked as resolved by gvanrossum

This comment has been minimized.

@gvanrossum

gvanrossum Feb 5, 2020
Member

This assert ought to be placed before the definition of c.

This comment has been minimized.

@brandtbucher

brandtbucher Feb 5, 2020
Author Member

Hm, does this reasoning hold for the other assertions (below) as well? It seemed natural to group each pair of results since they evaluate the same.

This comment has been minimized.

@gvanrossum

gvanrossum Feb 5, 2020
Member

I see what you did now. Leave it alone then.

This comment has been minimized.

@brandtbucher

brandtbucher Feb 5, 2020
Author Member

I guess what you're getting at is that we want to make sure the construction of c doesn't have side-effects on a and b?

This comment has been minimized.

@gvanrossum

gvanrossum Feb 5, 2020
Member

Not really, I just wanted a logical grouping of the tests -- but your grouping is more logical.

self.assertEqual(c, {0: 0, 1: 1, 2: 2, 3: 3})

c = b.copy()
c |= a

self.assertEqual(b | a, {1: 1, 2: 1, 3: 3, 0: 0})
self.assertEqual(c, {1: 1, 2: 1, 3: 3, 0: 0})

c = a.copy()
c |= [(1, 1), (2, 2), (3, 3)]

self.assertEqual(c, {0: 0, 1: 1, 2: 2, 3: 3})

self.assertIs(a.__or__(None), NotImplemented)
self.assertIs(a.__or__(()), NotImplemented)
self.assertIs(a.__or__("BAD"), NotImplemented)
self.assertIs(a.__or__(""), NotImplemented)

self.assertRaises(TypeError, a.__ior__, None)
self.assertEqual(a.__ior__(()), {0: 0, 1: 1, 2: 1})
self.assertRaises(ValueError, a.__ior__, "BAD")
This conversation was marked as resolved by brandtbucher

This comment has been minimized.

@serhiy-storchaka

serhiy-storchaka Feb 5, 2020
Member

What about a.__ior__("")?

This comment has been minimized.

@brandtbucher

brandtbucher Feb 5, 2020
Author Member

Thanks, added (and a.__or__(""), too).

self.assertEqual(a.__ior__(""), {0: 0, 1: 1, 2: 1})

def test_bool(self):
self.assertIs(not {}, True)
self.assertTrue({1: 2})
@@ -0,0 +1,2 @@
:class:`dict` (and :class:`collections.UserDict`) objects now support PEP 584's merge (``|``) and update (``|=``) operators.
Patch by Brandt Bucher.
@@ -2310,6 +2310,25 @@ dict_fromkeys_impl(PyTypeObject *type, PyObject *iterable, PyObject *value)
return _PyDict_FromKeys((PyObject *)type, iterable, value);
}

/* Single-arg dict update; used by dict_update_common and operators. */
static int
dict_update_arg(PyObject *self, PyObject *arg)
{
if (PyDict_CheckExact(arg)) {
return PyDict_Merge(self, arg, 1);
}
_Py_IDENTIFIER(keys);
PyObject *func;
if (_PyObject_LookupAttrId(arg, &PyId_keys, &func) < 0) {
return -1;
}
if (func != NULL) {
Py_DECREF(func);
return PyDict_Merge(self, arg, 1);
}
return PyDict_MergeFromSeq2(self, arg, 1);
}

static int
dict_update_common(PyObject *self, PyObject *args, PyObject *kwds,
const char *methname)
@@ -2321,23 +2340,7 @@ dict_update_common(PyObject *self, PyObject *args, PyObject *kwds,
result = -1;
}
else if (arg != NULL) {
if (PyDict_CheckExact(arg)) {
result = PyDict_Merge(self, arg, 1);
}
else {
_Py_IDENTIFIER(keys);
PyObject *func;
if (_PyObject_LookupAttrId(arg, &PyId_keys, &func) < 0) {
result = -1;
}
else if (func != NULL) {
Py_DECREF(func);
result = PyDict_Merge(self, arg, 1);
}
else {
result = PyDict_MergeFromSeq2(self, arg, 1);
}
}
result = dict_update_arg(self, arg);
}

if (result == 0 && kwds != NULL) {
@@ -3157,6 +3160,33 @@ dict_sizeof(PyDictObject *mp, PyObject *Py_UNUSED(ignored))
return PyLong_FromSsize_t(_PyDict_SizeOf(mp));
}

static PyObject *
dict_or(PyObject *self, PyObject *other)
{
if (!PyDict_Check(self) || !PyDict_Check(other)) {
Py_RETURN_NOTIMPLEMENTED;
}
PyObject *new = PyDict_Copy(self);
if (new == NULL) {
return NULL;
}
if (dict_update_arg(new, other)) {
Py_DECREF(new);
return NULL;
}
return new;
}

static PyObject *
dict_ior(PyObject *self, PyObject *other)
{
if (dict_update_arg(self, other)) {
return NULL;
}
Py_INCREF(self);
return self;
}

PyDoc_STRVAR(getitem__doc__, "x.__getitem__(y) <==> x[y]");

PyDoc_STRVAR(sizeof__doc__,
@@ -3262,6 +3292,11 @@ static PySequenceMethods dict_as_sequence = {
0, /* sq_inplace_repeat */
};

static PyNumberMethods dict_as_number = {
.nb_or = dict_or,
.nb_inplace_or = dict_ior,
};

static PyObject *
dict_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
{
@@ -3323,7 +3358,7 @@ PyTypeObject PyDict_Type = {
0, /* tp_setattr */
0, /* tp_as_async */
(reprfunc)dict_repr, /* tp_repr */
0, /* tp_as_number */
&dict_as_number, /* tp_as_number */
&dict_as_sequence, /* tp_as_sequence */
&dict_as_mapping, /* tp_as_mapping */
PyObject_HashNotImplemented, /* tp_hash */
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.