From 9d0723042f8041058ef210001849d3da5c138aca Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Mon, 5 Feb 2018 10:37:42 +0100 Subject: [PATCH 1/7] Retain order if these or make_class are passed an ordered dict Fixes #339 --- .gitignore | 3 ++- changelog.d/339.change.rst | 3 +++ src/attr/_compat.py | 7 +++++++ src/attr/_make.py | 18 +++++++++++++++--- tests/test_make.py | 39 ++++++++++++++++++++++++++++++++------ 5 files changed, 60 insertions(+), 10 deletions(-) create mode 100644 changelog.d/339.change.rst diff --git a/.gitignore b/.gitignore index a4ca3f8ce..af0920e6a 100644 --- a/.gitignore +++ b/.gitignore @@ -6,4 +6,5 @@ docs/_build/ htmlcov dist .cache -.hypothesis \ No newline at end of file +.hypothesis +.pytest_cache diff --git a/changelog.d/339.change.rst b/changelog.d/339.change.rst new file mode 100644 index 000000000..6a090ac20 --- /dev/null +++ b/changelog.d/339.change.rst @@ -0,0 +1,3 @@ +The order of attributes that are passed into ``attr.make_class()`` or the ``these`` argument of ``@attr.s()`` is now retained if the dictionary is ordered (i.e. ``dict`` on Python 3.6 and later, ``collections.OrderedDict`` otherwise). + +Before, the order was always determined by the order in which the attributes have been defined which may not be desirable when creating classes programatically. diff --git a/src/attr/_compat.py b/src/attr/_compat.py index b0d9edbb2..ad62e3afb 100644 --- a/src/attr/_compat.py +++ b/src/attr/_compat.py @@ -5,11 +5,15 @@ import types import warnings +from collections import OrderedDict + PY2 = sys.version_info[0] == 2 PYPY = platform.python_implementation() == "PyPy" +ordered_dict = OrderedDict # gets overwritten on Python 3.6+. + if PY2: from UserDict import IterableUserDict @@ -75,6 +79,9 @@ def metadata_proxy(d): return res else: + if sys.version_info[1] >= 6: + ordered_dict = dict + def isclass(klass): return isinstance(klass, type) diff --git a/src/attr/_make.py b/src/attr/_make.py index 7515eccd3..7cc401b81 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -8,7 +8,9 @@ from operator import itemgetter from . import _config -from ._compat import PY2, isclass, iteritems, metadata_proxy, set_closure_cell +from ._compat import ( + PY2, isclass, iteritems, metadata_proxy, ordered_dict, set_closure_cell +) from .exceptions import ( DefaultAlreadySetError, FrozenInstanceError, NotAnAttrsClassError, UnannotatedAttributeError @@ -233,6 +235,13 @@ def _get_annotations(cls): return anns +def _counter_getter(e): + """ + Key function for sorting to avoid re-creating a lambda for every class. + """ + return e[1].counter + + def _transform_attrs(cls, these, auto_attribs): """ Transform all `_CountingAttr`s on a class into `Attribute`s. @@ -245,11 +254,14 @@ def _transform_attrs(cls, these, auto_attribs): anns = _get_annotations(cls) if these is not None: - ca_list = sorted(( + ca_list = [ (name, ca) for name, ca in iteritems(these) - ), key=lambda e: e[1].counter) + ] + + if not isinstance(these, ordered_dict): + ca_list.sort(key=_counter_getter) elif auto_attribs is True: ca_names = { name diff --git a/tests/test_make.py b/tests/test_make.py index 64bcbb23d..4b6033e48 100644 --- a/tests/test_make.py +++ b/tests/test_make.py @@ -19,7 +19,7 @@ import attr from attr import _config -from attr._compat import PY2 +from attr._compat import PY2, ordered_dict from attr._make import ( Attribute, Factory, _AndValidator, _Attributes, _ClassBuilder, _CountingAttr, _transform_attrs, and_, fields, make_class, validate @@ -281,6 +281,20 @@ class C(object): assert 5 == C().x assert "C(x=5)" == repr(C()) + def test_these_ordered(self): + """ + If these is passed ordered attrs, their order respect instead of the + counter. + """ + b = attr.ib(default=2) + a = attr.ib(default=1) + + @attr.s(these=ordered_dict([("a", a), ("b", b)])) + class C(object): + pass + + assert "C(a=1, b=2)" == repr(C()) + def test_multiple_inheritance(self): """ Order of attributes doesn't get mixed up by multiple inheritance. @@ -610,6 +624,18 @@ def test_missing_sys_getframe(self, monkeypatch): assert 1 == len(C.__attrs_attrs__) + def test_make_class_ordered(self): + """ + If `make_class()` is passed ordered attrs, their order is respected + instead of the counter. + """ + b = attr.ib(default=2) + a = attr.ib(default=1) + + C = attr.make_class("C", ordered_dict([("a", a), ("b", b)])) + + assert "C(a=1, b=2)" == repr(C()) + class TestFields(object): """ @@ -686,13 +712,14 @@ def test_convert_factory_property(self, val, init): """ Property tests for attributes with convert, and a factory default. """ - C = make_class("C", { - "y": attr.ib(), - "x": attr.ib( + C = make_class("C", ordered_dict([ + ("y", attr.ib()), + ("x", attr.ib( init=init, default=Factory(lambda: val), - converter=lambda v: v + 1), - }) + converter=lambda v: v + 1 + )), + ])) c = C(2) assert c.x == val + 1 From 05226bd0160a8b4bb2a0d30620bf3b23fb494fac Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Mon, 5 Feb 2018 10:41:23 +0100 Subject: [PATCH 2/7] Add newsfragment --- changelog.d/343.change.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog.d/343.change.rst diff --git a/changelog.d/343.change.rst b/changelog.d/343.change.rst new file mode 100644 index 000000000..6a090ac20 --- /dev/null +++ b/changelog.d/343.change.rst @@ -0,0 +1,3 @@ +The order of attributes that are passed into ``attr.make_class()`` or the ``these`` argument of ``@attr.s()`` is now retained if the dictionary is ordered (i.e. ``dict`` on Python 3.6 and later, ``collections.OrderedDict`` otherwise). + +Before, the order was always determined by the order in which the attributes have been defined which may not be desirable when creating classes programatically. From f5477a75828ce65ff16f21362a9f100c2dab482a Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Mon, 5 Feb 2018 10:52:07 +0100 Subject: [PATCH 3/7] Another newsfragment Fixes #300 --- changelog.d/300.change.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog.d/300.change.rst diff --git a/changelog.d/300.change.rst b/changelog.d/300.change.rst new file mode 100644 index 000000000..6a090ac20 --- /dev/null +++ b/changelog.d/300.change.rst @@ -0,0 +1,3 @@ +The order of attributes that are passed into ``attr.make_class()`` or the ``these`` argument of ``@attr.s()`` is now retained if the dictionary is ordered (i.e. ``dict`` on Python 3.6 and later, ``collections.OrderedDict`` otherwise). + +Before, the order was always determined by the order in which the attributes have been defined which may not be desirable when creating classes programatically. From 29c9180cfc54696a435eff982f921c81901db77b Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Mon, 5 Feb 2018 11:27:16 +0100 Subject: [PATCH 4/7] Docs and change tags --- src/attr/_make.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/attr/_make.py b/src/attr/_make.py index 7cc401b81..14188bb8d 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -605,6 +605,11 @@ def attrs(maybe_cls=None, these=None, repr_ns=None, If *these* is not ``None``, ``attrs`` will *not* search the class body for attributes and will *not* remove any attributes from it. + If *these* is an ordered dict (:class:`dict` on Python 3.6+, + :class:`collections.OrderedDict` otherwise), the order is deduced from + the order of the attributes inside *these*. Otherwise the order + of the definition of the attributes is used. + :type these: :class:`dict` of :class:`str` to :func:`attr.ib` :param str repr_ns: When using nested classes, there's no way in Python 2 @@ -693,6 +698,7 @@ def attrs(maybe_cls=None, these=None, repr_ns=None, .. versionadded:: 17.3.0 *auto_attribs* .. versionchanged:: 18.1.0 If *these* is passed, no attributes are deleted from the class body. + .. versionchanged:: 18.1.0 If *these* is ordered, the order is retained. """ def wrap(cls): if getattr(cls, "__class__", None) is None: @@ -1525,6 +1531,11 @@ def make_class(name, attrs, bases=(object,), **attributes_arguments): :param attrs: A list of names or a dictionary of mappings of names to attributes. + + If *attrs* is a list or an ordered dict (:class:`dict` on Python 3.6+, + :class:`collections.OrderedDict` otherwise), the order is deduced from + the order of the names or attributes inside *attrs*. Otherwise the + order of the definition of the attributes is used. :type attrs: :class:`list` or :class:`dict` :param tuple bases: Classes that the new class will subclass. @@ -1534,7 +1545,8 @@ def make_class(name, attrs, bases=(object,), **attributes_arguments): :return: A new class with *attrs*. :rtype: type - .. versionadded:: 17.1.0 *bases* + .. versionadded:: 17.1.0 *bases* + .. versionchanged:: 18.1.0 If *attrs* is ordered, the order is retained. """ if isinstance(attrs, dict): cls_dict = attrs From 05ac6eb86c786402a9c1db69deeb60864dd3062d Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Mon, 5 Feb 2018 11:51:45 +0100 Subject: [PATCH 5/7] Disable coverage and explain why --- src/attr/_compat.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/attr/_compat.py b/src/attr/_compat.py index ad62e3afb..d1936e416 100644 --- a/src/attr/_compat.py +++ b/src/attr/_compat.py @@ -79,7 +79,9 @@ def metadata_proxy(d): return res else: - if sys.version_info[1] >= 6: + # Disabling coverage because we run the test suite under coverage only for + # the latest Python 3. + if sys.version_info[1] >= 6: # pragma: nocover ordered_dict = dict def isclass(klass): From 4c41441ea5d711713098c946594e748966d443d8 Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Mon, 5 Feb 2018 17:02:57 +0100 Subject: [PATCH 6/7] PyPy has ordered dicts too --- src/attr/_compat.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/attr/_compat.py b/src/attr/_compat.py index d1936e416..ccd1b9463 100644 --- a/src/attr/_compat.py +++ b/src/attr/_compat.py @@ -12,7 +12,11 @@ PYPY = platform.python_implementation() == "PyPy" -ordered_dict = OrderedDict # gets overwritten on Python 3.6+. +if PYPY or sys.version_info[:2] >= (3, 6): + ordered_dict = dict +else: + ordered_dict = OrderedDict + if PY2: from UserDict import IterableUserDict @@ -79,11 +83,6 @@ def metadata_proxy(d): return res else: - # Disabling coverage because we run the test suite under coverage only for - # the latest Python 3. - if sys.version_info[1] >= 6: # pragma: nocover - ordered_dict = dict - def isclass(klass): return isinstance(klass, type) From 5e8a21e2367c67e7f1cf348762a60e32d1f1d06d Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Mon, 5 Feb 2018 17:04:41 +0100 Subject: [PATCH 7/7] Import OrderedDict only when necessary --- src/attr/_compat.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/attr/_compat.py b/src/attr/_compat.py index ccd1b9463..42a91ee5d 100644 --- a/src/attr/_compat.py +++ b/src/attr/_compat.py @@ -5,8 +5,6 @@ import types import warnings -from collections import OrderedDict - PY2 = sys.version_info[0] == 2 PYPY = platform.python_implementation() == "PyPy" @@ -15,6 +13,7 @@ if PYPY or sys.version_info[:2] >= (3, 6): ordered_dict = dict else: + from collections import OrderedDict ordered_dict = OrderedDict