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/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. 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/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. diff --git a/src/attr/_compat.py b/src/attr/_compat.py index b0d9edbb2..42a91ee5d 100644 --- a/src/attr/_compat.py +++ b/src/attr/_compat.py @@ -10,6 +10,13 @@ PYPY = platform.python_implementation() == "PyPy" +if PYPY or sys.version_info[:2] >= (3, 6): + ordered_dict = dict +else: + from collections import OrderedDict + ordered_dict = OrderedDict + + if PY2: from UserDict import IterableUserDict diff --git a/src/attr/_make.py b/src/attr/_make.py index 7515eccd3..14188bb8d 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 @@ -593,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 @@ -681,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: @@ -1513,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. @@ -1522,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 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