From 253f5dbefb522632e4b2c44a708512233f97b3c3 Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Thu, 2 Aug 2018 22:35:03 +0900 Subject: [PATCH 1/4] bpo-34320: Fix dict(od) didn't copy order --- Lib/test/test_ordered_dict.py | 37 +++++++++++++++++++ .../2018-08-02-22-34-59.bpo-34320.hNshAA.rst | 1 + Objects/dictobject.c | 4 +- 3 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-08-02-22-34-59.bpo-34320.hNshAA.rst diff --git a/Lib/test/test_ordered_dict.py b/Lib/test/test_ordered_dict.py index 20efe3729de838..410ed2f7cf9b31 100644 --- a/Lib/test/test_ordered_dict.py +++ b/Lib/test/test_ordered_dict.py @@ -650,6 +650,43 @@ 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_kwargs_order(self): + # bpo-34320 + od = self.OrderedDict([('a', 1), ('b', 2)]) + od.move_to_end('a') + expected = list(od.items()) + + def fn(**kw): + return kw + + res = fn(**od) + self.assertIsInstance(res, dict) + self.assertEqual(list(res.items()), expected) + + def test_type_namespace_order(self): + # bpo-34320 + od = self.OrderedDict([('a', 1), ('b', 2)]) + od.move_to_end('a') + expected = list(od.items()) + + C = type('C', (), od) + self.assertEqual(list(C.__dict__.items())[:2], [('b', 2), ('a', 1)]) + + def test_dict_copy_order(self): + # Issue 34320 + od = self.OrderedDict([('a', 1), ('b', 2)]) + od.move_to_end('a') + expected = list(od.items()) + + copy = dict(od) + self.assertEqual(list(copy.items()), expected) + + # Namespace preserves order by language spec from 3.6. + # Dict preserves order by language spec from 3.7. + # Dict preserves order by implementation from 3.6. + if sys.version_info < (3, 7): + test_dict_copy_order = support.cpython_only(test_dict_copy_order) + class PurePythonOrderedDictTests(OrderedDictTests, unittest.TestCase): diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-08-02-22-34-59.bpo-34320.hNshAA.rst b/Misc/NEWS.d/next/Core and Builtins/2018-08-02-22-34-59.bpo-34320.hNshAA.rst new file mode 100644 index 00000000000000..ce5b3397e65fa7 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-08-02-22-34-59.bpo-34320.hNshAA.rst @@ -0,0 +1 @@ +Fix ``dict(od)`` didn't copy iteration order of OrderedDict. diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 413557d6674127..fec69678c50066 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -235,6 +235,8 @@ static Py_ssize_t lookdict_split(PyDictObject *mp, PyObject *key, static int dictresize(PyDictObject *mp, Py_ssize_t minused); +static PyObject* dict_iter(PyDictObject *dict); + /*Global counter used to set ma_version_tag field of dictionary. * It is incremented each time that a dictionary is created and each * time that a dictionary is modified. */ @@ -2379,7 +2381,7 @@ dict_merge(PyObject *a, PyObject *b, int override) return -1; } mp = (PyDictObject*)a; - if (PyDict_Check(b)) { + if (PyDict_Check(b) && (Py_TYPE(b)->tp_iter == (getiterfunc)dict_iter)) { other = (PyDictObject*)b; if (other == mp || other->ma_used == 0) /* a.update(a) or a.update({}); nothing to do */ From e0bf46cd62f51ab5798ad3c6bd656b2ddf962f5a Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Thu, 20 Sep 2018 19:13:03 +0900 Subject: [PATCH 2/4] Remove unnecessary version check --- Lib/test/test_ordered_dict.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_ordered_dict.py b/Lib/test/test_ordered_dict.py index 410ed2f7cf9b31..7cc764e6a565e5 100644 --- a/Lib/test/test_ordered_dict.py +++ b/Lib/test/test_ordered_dict.py @@ -650,6 +650,7 @@ 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) + # TODO: Move this test to more appropriate place. def test_kwargs_order(self): # bpo-34320 od = self.OrderedDict([('a', 1), ('b', 2)]) @@ -663,6 +664,7 @@ def fn(**kw): self.assertIsInstance(res, dict) self.assertEqual(list(res.items()), expected) + # TODO: Move this test to more appropriate place. def test_type_namespace_order(self): # bpo-34320 od = self.OrderedDict([('a', 1), ('b', 2)]) @@ -673,7 +675,7 @@ def test_type_namespace_order(self): self.assertEqual(list(C.__dict__.items())[:2], [('b', 2), ('a', 1)]) def test_dict_copy_order(self): - # Issue 34320 + # bpo-34320 od = self.OrderedDict([('a', 1), ('b', 2)]) od.move_to_end('a') expected = list(od.items()) @@ -681,12 +683,6 @@ def test_dict_copy_order(self): copy = dict(od) self.assertEqual(list(copy.items()), expected) - # Namespace preserves order by language spec from 3.6. - # Dict preserves order by language spec from 3.7. - # Dict preserves order by implementation from 3.6. - if sys.version_info < (3, 7): - test_dict_copy_order = support.cpython_only(test_dict_copy_order) - class PurePythonOrderedDictTests(OrderedDictTests, unittest.TestCase): From 26ef93f365e1159abb1abe44364a0de7c472914e Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Thu, 20 Sep 2018 19:18:13 +0900 Subject: [PATCH 3/4] Update comment --- Lib/test/test_ordered_dict.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_ordered_dict.py b/Lib/test/test_ordered_dict.py index 7cc764e6a565e5..ef5dbb3fb3137a 100644 --- a/Lib/test/test_ordered_dict.py +++ b/Lib/test/test_ordered_dict.py @@ -650,7 +650,8 @@ 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) - # TODO: Move this test to more appropriate place. + # TODO: This test is relating to PEP 468. + # Move this test to somewhere more appropriate. def test_kwargs_order(self): # bpo-34320 od = self.OrderedDict([('a', 1), ('b', 2)]) @@ -664,7 +665,8 @@ def fn(**kw): self.assertIsInstance(res, dict) self.assertEqual(list(res.items()), expected) - # TODO: Move this test to more appropriate place. + # TODO: This test is relating to PEP 520. + # Move this test to somewhere more appropriate. def test_type_namespace_order(self): # bpo-34320 od = self.OrderedDict([('a', 1), ('b', 2)]) From ee8696d1019f92d3f3e191b848fbbe15e1173927 Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Tue, 25 Sep 2018 20:38:36 +0900 Subject: [PATCH 4/4] Move tests --- Lib/test/test_builtin.py | 9 +++++++++ Lib/test/test_call.py | 17 +++++++++++++++++ Lib/test/test_dict.py | 30 ++++++++++++++++++++++++++++++ Lib/test/test_ordered_dict.py | 35 ----------------------------------- 4 files changed, 56 insertions(+), 35 deletions(-) diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index 46cf2d328362fb..dafcf004c09568 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -1968,6 +1968,15 @@ class B: with self.assertRaises(TypeError): type('A', (B,), {'__slots__': '__weakref__'}) + def test_namespace_order(self): + # bpo-34320: namespace should preserve order + od = collections.OrderedDict([('a', 1), ('b', 2)]) + od.move_to_end('a') + expected = list(od.items()) + + C = type('C', (), od) + self.assertEqual(list(C.__dict__.items())[:2], [('b', 2), ('a', 1)]) + def load_tests(loader, tests, pattern): from doctest import DocTestSuite diff --git a/Lib/test/test_call.py b/Lib/test/test_call.py index ec9697a444f0d9..45b34e46a5abb2 100644 --- a/Lib/test/test_call.py +++ b/Lib/test/test_call.py @@ -9,6 +9,23 @@ import collections import itertools + +class FunctionCalls(unittest.TestCase): + + def test_kwargs_order(self): + # bpo-34320: **kwargs should preserve order of passed OrderedDict + od = collections.OrderedDict([('a', 1), ('b', 2)]) + od.move_to_end('a') + expected = list(od.items()) + + def fn(**kw): + return kw + + res = fn(**od) + self.assertIsInstance(res, dict) + self.assertEqual(list(res.items()), expected) + + # The test cases here cover several paths through the function calling # code. They depend on the METH_XXX flag that is used to define a C # function, which can't be verified from Python. If the METH_XXX decl diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index 38521bbf663052..90c0a3131a78eb 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -1222,6 +1222,36 @@ def iter_and_mutate(): self.assertRaises(RuntimeError, iter_and_mutate) + def test_dict_copy_order(self): + # bpo-34320 + od = collections.OrderedDict([('a', 1), ('b', 2)]) + od.move_to_end('a') + expected = list(od.items()) + + copy = dict(od) + self.assertEqual(list(copy.items()), expected) + + # dict subclass doesn't override __iter__ + class CustomDict(dict): + pass + + pairs = [('a', 1), ('b', 2), ('c', 3)] + + d = CustomDict(pairs) + self.assertEqual(pairs, list(dict(d).items())) + + class CustomReversedDict(dict): + def keys(self): + return reversed(list(dict.keys(self))) + + __iter__ = keys + + def items(self): + return reversed(dict.items(self)) + + d = CustomReversedDict(pairs) + self.assertEqual(pairs[::-1], list(dict(d).items())) + class CAPITest(unittest.TestCase): diff --git a/Lib/test/test_ordered_dict.py b/Lib/test/test_ordered_dict.py index ef5dbb3fb3137a..20efe3729de838 100644 --- a/Lib/test/test_ordered_dict.py +++ b/Lib/test/test_ordered_dict.py @@ -650,41 +650,6 @@ 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) - # TODO: This test is relating to PEP 468. - # Move this test to somewhere more appropriate. - def test_kwargs_order(self): - # bpo-34320 - od = self.OrderedDict([('a', 1), ('b', 2)]) - od.move_to_end('a') - expected = list(od.items()) - - def fn(**kw): - return kw - - res = fn(**od) - self.assertIsInstance(res, dict) - self.assertEqual(list(res.items()), expected) - - # TODO: This test is relating to PEP 520. - # Move this test to somewhere more appropriate. - def test_type_namespace_order(self): - # bpo-34320 - od = self.OrderedDict([('a', 1), ('b', 2)]) - od.move_to_end('a') - expected = list(od.items()) - - C = type('C', (), od) - self.assertEqual(list(C.__dict__.items())[:2], [('b', 2), ('a', 1)]) - - def test_dict_copy_order(self): - # bpo-34320 - od = self.OrderedDict([('a', 1), ('b', 2)]) - od.move_to_end('a') - expected = list(od.items()) - - copy = dict(od) - self.assertEqual(list(copy.items()), expected) - class PurePythonOrderedDictTests(OrderedDictTests, unittest.TestCase):