From 74d6bc3b87f3e178a0010baaa1921105640e0057 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 27 Feb 2020 19:43:13 -0800 Subject: [PATCH 01/11] Add number methods. --- Objects/odictobject.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/Objects/odictobject.c b/Objects/odictobject.c index f412220e8cc02c..fdde7090dccf7c 100644 --- a/Objects/odictobject.c +++ b/Objects/odictobject.c @@ -851,6 +851,30 @@ static PyMappingMethods odict_as_mapping = { }; +/* ---------------------------------------------- + * OrderedDict number methods + */ + +static PyObject * +odict_or(PyObject *left, PyObject *right) +{ + // TODO +} + +static PyObject * +odict_inplace_or(PyODictObject *self, PyObject *other) +{ + // TODO +} + +/* tp_as_number */ + +static PyNumberMethods odict_as_number = { + .nb_or = odict_or, + .nb_inplace_or = odict_inplace_or, +}; + + /* ---------------------------------------------- * OrderedDict methods */ @@ -1557,7 +1581,7 @@ PyTypeObject PyODict_Type = { 0, /* tp_setattr */ 0, /* tp_as_async */ (reprfunc)odict_repr, /* tp_repr */ - 0, /* tp_as_number */ + &odict_as_number, /* tp_as_number */ 0, /* tp_as_sequence */ &odict_as_mapping, /* tp_as_mapping */ 0, /* tp_hash */ From 2c4f952848db6d686eda217279d7a3ce3e7bc2f6 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sat, 7 Mar 2020 16:41:19 -0800 Subject: [PATCH 02/11] Add pure-Python implementation. --- Lib/collections/__init__.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Lib/collections/__init__.py b/Lib/collections/__init__.py index 1aa7d10ad22e4a..d4346ea8fb925c 100644 --- a/Lib/collections/__init__.py +++ b/Lib/collections/__init__.py @@ -293,6 +293,24 @@ def __eq__(self, other): return dict.__eq__(self, other) and all(map(_eq, self, other)) return dict.__eq__(self, other) + def __ior__(self, other): + self.update(other) + return self + + def __or__(self, other): + if not isinstance(other, dict): + return NotImplemented + new = self.copy() + new.update(other) + return new + + def __ror__(self, other): + if not isinstance(other, dict): + return NotImplemented + new = self.__class__(other) + new.update(self) + return new + try: from _collections import OrderedDict From 6129a4bd07144a85a236877d0a7fa9c17c797286 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 11 Mar 2020 09:06:14 -0700 Subject: [PATCH 03/11] Implement operators. --- Objects/odictobject.c | 186 ++++++++++++++++++++++++------------------ 1 file changed, 107 insertions(+), 79 deletions(-) diff --git a/Objects/odictobject.c b/Objects/odictobject.c index fdde7090dccf7c..b919c57ff87870 100644 --- a/Objects/odictobject.c +++ b/Objects/odictobject.c @@ -858,13 +858,42 @@ static PyMappingMethods odict_as_mapping = { static PyObject * odict_or(PyObject *left, PyObject *right) { - // TODO + if (PyODict_Check(left)) { + self = left; + other = right + } + else { + self = right; + other = left; + } + if (!PyDict_Check(other)) { + Py_RETURN_NOTIMPLEMENTED; + } + PyObject *new; + if (PyODict_CheckExact(self)) { + new = PyODict_New(); + } + else { + new = _PyObject_CallNoArg(Py_TYPE(self)); + } + if (!new) { + return NULL; + } + if (mutablemapping_update_arg(new, left) < 0 || mutablemapping_update_arg(new, right) < 0) { + Py_DECREF(new); + return NULL; + } + return new; } static PyObject * -odict_inplace_or(PyODictObject *self, PyObject *other) +odict_inplace_or(PyObject *self, PyObject *other) { - // TODO + if (mutablemapping_update_arg(self, other) < 0) { + return NULL; + } + Py_INCREF(self); + return self; } /* tp_as_number */ @@ -2215,16 +2244,84 @@ mutablemapping_add_pairs(PyObject *self, PyObject *pairs) return 0; } -static PyObject * -mutablemapping_update(PyObject *self, PyObject *args, PyObject *kwargs) +static int +mutablemapping_update_arg(PyObject *self, PyObject *arg) { - int res = 0; - Py_ssize_t len; + int res; + if (PyDict_CheckExact(arg)) { + PyObject *items = PyDict_Items(arg); + Py_DECREF(arg); + if (items == NULL) { + return -1; + } + res = mutablemapping_add_pairs(self, items); + Py_DECREF(items); + return res; + } _Py_IDENTIFIER(keys); + PyObject *func; + if (_PyObject_LookupAttrId(arg, &PyId_keys, &func) < 0) { + Py_DECREF(arg); + return -1; + } + if (func != NULL) { + PyObject *keys = _PyObject_CallNoArg(func); + Py_DECREF(func); + if (keys == NULL) { + Py_DECREF(arg); + return -1; + } + PyObject *iterator = PyObject_GetIter(keys); + Py_DECREF(keys); + if (iterator == NULL) { + Py_DECREF(arg); + return -1; + } + PyObject *key; + while (res == 0 && (key = PyIter_Next(iterator))) { + PyObject *value = PyObject_GetItem(arg, key); + if (value != NULL) { + res = PyObject_SetItem(self, key, value); + Py_DECREF(value); + } + else { + res = -1; + } + Py_DECREF(key); + } + Py_DECREF(arg); + Py_DECREF(iterator); + if (res != 0 || PyErr_Occurred()) { + return -1; + } + return 0; + } + if (_PyObject_LookupAttrId(arg, &PyId_items, &func) < 0) { + Py_DECREF(arg); + return -1; + } + if (func != NULL) { + Py_DECREF(arg); + PyObject *items = _PyObject_CallNoArg(func); + Py_DECREF(func); + if (items == NULL) { + return -1; + } + res = mutablemapping_add_pairs(self, items); + Py_DECREF(items); + return res; + } + res = mutablemapping_add_pairs(self, arg); + Py_DECREF(arg); + return res; +} +static PyObject * +mutablemapping_update(PyObject *self, PyObject *args, PyObject *kwargs) +{ /* first handle args, if any */ assert(args == NULL || PyTuple_Check(args)); - len = (args != NULL) ? PyTuple_GET_SIZE(args) : 0; + Py_ssize_t len = (args != NULL) ? PyTuple_GET_SIZE(args) : 0; if (len > 1) { const char *msg = "update() takes at most 1 positional argument (%zd given)"; PyErr_Format(PyExc_TypeError, msg, len); @@ -2232,90 +2329,21 @@ mutablemapping_update(PyObject *self, PyObject *args, PyObject *kwargs) } if (len) { - PyObject *func; PyObject *other = PyTuple_GET_ITEM(args, 0); /* borrowed reference */ assert(other != NULL); Py_INCREF(other); - if (PyDict_CheckExact(other)) { - PyObject *items = PyDict_Items(other); - Py_DECREF(other); - if (items == NULL) - return NULL; - res = mutablemapping_add_pairs(self, items); - Py_DECREF(items); - if (res == -1) - return NULL; - goto handle_kwargs; - } - - if (_PyObject_LookupAttrId(other, &PyId_keys, &func) < 0) { - Py_DECREF(other); + if (mutablemapping_add_pairs(self, other) < 0) { return NULL; } - if (func != NULL) { - PyObject *keys, *iterator, *key; - keys = _PyObject_CallNoArg(func); - Py_DECREF(func); - if (keys == NULL) { - Py_DECREF(other); - return NULL; - } - iterator = PyObject_GetIter(keys); - Py_DECREF(keys); - if (iterator == NULL) { - Py_DECREF(other); - return NULL; - } - while (res == 0 && (key = PyIter_Next(iterator))) { - PyObject *value = PyObject_GetItem(other, key); - if (value != NULL) { - res = PyObject_SetItem(self, key, value); - Py_DECREF(value); - } - else { - res = -1; - } - Py_DECREF(key); - } - Py_DECREF(other); - Py_DECREF(iterator); - if (res != 0 || PyErr_Occurred()) - return NULL; - goto handle_kwargs; - } - - if (_PyObject_LookupAttrId(other, &PyId_items, &func) < 0) { - Py_DECREF(other); - return NULL; - } - if (func != NULL) { - PyObject *items; - Py_DECREF(other); - items = _PyObject_CallNoArg(func); - Py_DECREF(func); - if (items == NULL) - return NULL; - res = mutablemapping_add_pairs(self, items); - Py_DECREF(items); - if (res == -1) - return NULL; - goto handle_kwargs; - } - - res = mutablemapping_add_pairs(self, other); - Py_DECREF(other); - if (res != 0) - return NULL; } - handle_kwargs: /* now handle kwargs */ assert(kwargs == NULL || PyDict_Check(kwargs)); if (kwargs != NULL && PyDict_GET_SIZE(kwargs)) { PyObject *items = PyDict_Items(kwargs); if (items == NULL) return NULL; - res = mutablemapping_add_pairs(self, items); + int res = mutablemapping_add_pairs(self, items); Py_DECREF(items); if (res == -1) return NULL; From a89c47573ef759b9ab6d05d174b0055ee811aa70 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 12 Mar 2020 09:09:13 -0700 Subject: [PATCH 04/11] Add tests. --- Lib/test/test_ordered_dict.py | 46 +++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/Lib/test/test_ordered_dict.py b/Lib/test/test_ordered_dict.py index f337be8eb8551f..652f235d3f6230 100644 --- a/Lib/test/test_ordered_dict.py +++ b/Lib/test/test_ordered_dict.py @@ -654,6 +654,52 @@ def test_free_after_iterating(self): support.check_free_after_iterating(self, lambda d: iter(d.values()), self.OrderedDict) support.check_free_after_iterating(self, lambda d: iter(d.items()), self.OrderedDict) + def test_merge_operator(self): + + OrderedDict = self.OrderedDict + + a = OrderedDict({0: 0, 1: 1, 2: 1}) + b = OrderedDict({1: 1, 2: 2, 3: 3}) + + c = a.copy() + d = a.copy() + c |= b + d |= list(b.items()) + + expected = OrderedDict({0: 0, 1: 1, 2: 2, 3: 3}) + self.assertEqual(a | dict(b), expected) + self.assertEqual(a | b, expected) + self.assertEqual(c, expected) + self.assertEqual(d, expected) + + c = b.copy() + c |= a + + expected = OrderedDict({1: 1, 2: 1, 3: 3, 0: 0}) + self.assertEqual(dict(b) | a, expected) + self.assertEqual(b | a, expected) + self.assertEqual(c, expected) + + expected = a.copy() + + a |= () + self.assertEqual(a, expected) + + a |= "" + self.assertEqual(a, expected) + + with self.assertRaises(TypeError): + a | None + with self.assertRaises(TypeError): + a | () + with self.assertRaises(TypeError): + a | "BAD" + with self.assertRaises(TypeError): + a | "" + + with self.assertRaises(ValueError): + a |= "BAD" + class PurePythonOrderedDictTests(OrderedDictTests, unittest.TestCase): From 29ab4f654172caa639eb3952968e6f2ecc3eec51 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 12 Mar 2020 09:09:24 -0700 Subject: [PATCH 05/11] Clean up implementation. --- Objects/odictobject.c | 37 +++++++++++++++---------------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/Objects/odictobject.c b/Objects/odictobject.c index b919c57ff87870..6a412658941edf 100644 --- a/Objects/odictobject.c +++ b/Objects/odictobject.c @@ -855,31 +855,29 @@ static PyMappingMethods odict_as_mapping = { * OrderedDict number methods */ +static int mutablemapping_update_arg(PyObject*, PyObject*); + static PyObject * odict_or(PyObject *left, PyObject *right) { + PyTypeObject *type; + PyObject *other; if (PyODict_Check(left)) { - self = left; - other = right + type = Py_TYPE(left); + other = right; } else { - self = right; + type = Py_TYPE(right); other = left; } if (!PyDict_Check(other)) { Py_RETURN_NOTIMPLEMENTED; } - PyObject *new; - if (PyODict_CheckExact(self)) { - new = PyODict_New(); - } - else { - new = _PyObject_CallNoArg(Py_TYPE(self)); - } + PyObject *new = PyObject_CallFunctionObjArgs((PyObject*)type, left, NULL); if (!new) { return NULL; } - if (mutablemapping_update_arg(new, left) < 0 || mutablemapping_update_arg(new, right) < 0) { + if (mutablemapping_update_arg(new, right) < 0) { Py_DECREF(new); return NULL; } @@ -2247,10 +2245,9 @@ mutablemapping_add_pairs(PyObject *self, PyObject *pairs) static int mutablemapping_update_arg(PyObject *self, PyObject *arg) { - int res; + int res = 0; if (PyDict_CheckExact(arg)) { PyObject *items = PyDict_Items(arg); - Py_DECREF(arg); if (items == NULL) { return -1; } @@ -2261,20 +2258,17 @@ mutablemapping_update_arg(PyObject *self, PyObject *arg) _Py_IDENTIFIER(keys); PyObject *func; if (_PyObject_LookupAttrId(arg, &PyId_keys, &func) < 0) { - Py_DECREF(arg); return -1; } if (func != NULL) { PyObject *keys = _PyObject_CallNoArg(func); Py_DECREF(func); if (keys == NULL) { - Py_DECREF(arg); return -1; } PyObject *iterator = PyObject_GetIter(keys); Py_DECREF(keys); if (iterator == NULL) { - Py_DECREF(arg); return -1; } PyObject *key; @@ -2289,7 +2283,6 @@ mutablemapping_update_arg(PyObject *self, PyObject *arg) } Py_DECREF(key); } - Py_DECREF(arg); Py_DECREF(iterator); if (res != 0 || PyErr_Occurred()) { return -1; @@ -2297,11 +2290,9 @@ mutablemapping_update_arg(PyObject *self, PyObject *arg) return 0; } if (_PyObject_LookupAttrId(arg, &PyId_items, &func) < 0) { - Py_DECREF(arg); return -1; } if (func != NULL) { - Py_DECREF(arg); PyObject *items = _PyObject_CallNoArg(func); Py_DECREF(func); if (items == NULL) { @@ -2312,13 +2303,13 @@ mutablemapping_update_arg(PyObject *self, PyObject *arg) return res; } res = mutablemapping_add_pairs(self, arg); - Py_DECREF(arg); return res; } static PyObject * mutablemapping_update(PyObject *self, PyObject *args, PyObject *kwargs) { + int res; /* first handle args, if any */ assert(args == NULL || PyTuple_Check(args)); Py_ssize_t len = (args != NULL) ? PyTuple_GET_SIZE(args) : 0; @@ -2332,7 +2323,9 @@ mutablemapping_update(PyObject *self, PyObject *args, PyObject *kwargs) PyObject *other = PyTuple_GET_ITEM(args, 0); /* borrowed reference */ assert(other != NULL); Py_INCREF(other); - if (mutablemapping_add_pairs(self, other) < 0) { + res = mutablemapping_update_arg(self, other); + Py_DECREF(other); + if (res < 0) { return NULL; } } @@ -2343,7 +2336,7 @@ mutablemapping_update(PyObject *self, PyObject *args, PyObject *kwargs) PyObject *items = PyDict_Items(kwargs); if (items == NULL) return NULL; - int res = mutablemapping_add_pairs(self, items); + res = mutablemapping_add_pairs(self, items); Py_DECREF(items); if (res == -1) return NULL; From 41fc896eca083b25986432c9b51a5fdbabb5f10d Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 12 Mar 2020 09:09:53 -0700 Subject: [PATCH 06/11] Align Python with C implementation. --- Lib/collections/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/collections/__init__.py b/Lib/collections/__init__.py index d4346ea8fb925c..18255da17591c8 100644 --- a/Lib/collections/__init__.py +++ b/Lib/collections/__init__.py @@ -300,7 +300,7 @@ def __ior__(self, other): def __or__(self, other): if not isinstance(other, dict): return NotImplemented - new = self.copy() + new = self.__class__(self) new.update(other) return new From 2ef2da040906d4b902250881e4ef6d8f3dc098b3 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 12 Mar 2020 11:45:03 -0700 Subject: [PATCH 07/11] Clean up vertical whitespace. --- Lib/test/test_ordered_dict.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/Lib/test/test_ordered_dict.py b/Lib/test/test_ordered_dict.py index 652f235d3f6230..10b124f7593cb7 100644 --- a/Lib/test/test_ordered_dict.py +++ b/Lib/test/test_ordered_dict.py @@ -655,7 +655,6 @@ def test_free_after_iterating(self): support.check_free_after_iterating(self, lambda d: iter(d.items()), self.OrderedDict) def test_merge_operator(self): - OrderedDict = self.OrderedDict a = OrderedDict({0: 0, 1: 1, 2: 1}) @@ -665,7 +664,6 @@ def test_merge_operator(self): d = a.copy() c |= b d |= list(b.items()) - expected = OrderedDict({0: 0, 1: 1, 2: 2, 3: 3}) self.assertEqual(a | dict(b), expected) self.assertEqual(a | b, expected) @@ -674,17 +672,13 @@ def test_merge_operator(self): c = b.copy() c |= a - expected = OrderedDict({1: 1, 2: 1, 3: 3, 0: 0}) self.assertEqual(dict(b) | a, expected) self.assertEqual(b | a, expected) self.assertEqual(c, expected) expected = a.copy() - a |= () - self.assertEqual(a, expected) - a |= "" self.assertEqual(a, expected) @@ -696,7 +690,6 @@ def test_merge_operator(self): a | "BAD" with self.assertRaises(TypeError): a | "" - with self.assertRaises(ValueError): a |= "BAD" From 6c4ca78bac55923420b361a606b212b94d6a9384 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 12 Mar 2020 11:55:40 -0700 Subject: [PATCH 08/11] Add NEWS entry. --- .../next/Library/2020-03-12-11-55-16.bpo-36144.9bxGH_.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2020-03-12-11-55-16.bpo-36144.9bxGH_.rst diff --git a/Misc/NEWS.d/next/Library/2020-03-12-11-55-16.bpo-36144.9bxGH_.rst b/Misc/NEWS.d/next/Library/2020-03-12-11-55-16.bpo-36144.9bxGH_.rst new file mode 100644 index 00000000000000..6cc35a21428fe0 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-03-12-11-55-16.bpo-36144.9bxGH_.rst @@ -0,0 +1,2 @@ +:class:`collections.OrderedDict` now implements ``|`` and ``|=`` +(:pep:`584`). From 13e3760e84bacc2458f34ccd6ad66bbc62306052 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 12 Mar 2020 11:58:35 -0700 Subject: [PATCH 09/11] Update docs. --- Doc/library/collections.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Doc/library/collections.rst b/Doc/library/collections.rst index 65cdf34aa4e4fe..f0cb6518945a1c 100644 --- a/Doc/library/collections.rst +++ b/Doc/library/collections.rst @@ -1119,6 +1119,10 @@ anywhere a regular dictionary is used. passed to the :class:`OrderedDict` constructor and its :meth:`update` method. +.. versionchanged:: 3.9 + Added merge (``|``) and update (``|=``) operators, specified in :pep:`584`. + + :class:`OrderedDict` Examples and Recipes ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From afe76f8c3ca6262915b59acae7cb70f4206fb11b Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 12 Mar 2020 22:54:10 -0700 Subject: [PATCH 10/11] PyObject_CallFunctionObjArgs -> PyObject_CallOneArg --- Objects/odictobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/odictobject.c b/Objects/odictobject.c index 6a412658941edf..befe2b12768988 100644 --- a/Objects/odictobject.c +++ b/Objects/odictobject.c @@ -873,7 +873,7 @@ odict_or(PyObject *left, PyObject *right) if (!PyDict_Check(other)) { Py_RETURN_NOTIMPLEMENTED; } - PyObject *new = PyObject_CallFunctionObjArgs((PyObject*)type, left, NULL); + PyObject *new = PyObject_CallOneArg((PyObject*)type, left); if (!new) { return NULL; } From 1544f2a936899b4402ae2e9448968e644320fb71 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 12 Mar 2020 22:56:51 -0700 Subject: [PATCH 11/11] Add type tests. --- Lib/test/test_ordered_dict.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Lib/test/test_ordered_dict.py b/Lib/test/test_ordered_dict.py index 10b124f7593cb7..5d3ffede35c661 100644 --- a/Lib/test/test_ordered_dict.py +++ b/Lib/test/test_ordered_dict.py @@ -677,6 +677,10 @@ def test_merge_operator(self): self.assertEqual(b | a, expected) self.assertEqual(c, expected) + self.assertIs(type(a | b), OrderedDict) + self.assertIs(type(dict(a) | b), OrderedDict) + self.assertIs(type(a | dict(b)), OrderedDict) + expected = a.copy() a |= () a |= ""