Repository: python-attrs/attrs

Problem statement:
Allow overwriting order inferred from _CountingAttr.counter when passing ordered `these`
attrs classes can be constructed dynamically using `attr.s(maybe_cls=A, these=some_dict)`.
Usually `some_dict` will be a standard (unordered) python dictionary and the order of attributes has to be inferred from `_CountingAttr.counter`.

What about allowing the user to overwrite the `_CountingAttr.counter` by passing an `OrderedDict` as argument to `these`?


Hints:
I think @Julian would like this?
Yep! Would love to see what this looks like.

Patch:
diff --git a/src/attr/_compat.py b/src/attr/_compat.py
--- 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
--- 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


Test patch:
diff --git a/tests/test_make.py b/tests/test_make.py
--- 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