Address review notes.

python-attrs · Tinche · Sep 17, 2017 · Aug 28, 2017 · Aug 30, 2017 · Aug 30, 2017
commit d10e1af95328ea9e7d7df1ea9bd15e700a1afdcc
diff --git a/changelog.d/239.change.rst b/changelog.d/239.change.rst
@@ -0,0 +1,3 @@
+Added ``type`` argument to ``attr.attr()`` and corresponding ``type`` attribute to ``attr.Attribute``.
+This value can be inspected by third-party tools for type checking and serialization.
+In Python 3.6 or higher, the value of ``attr.Attribute.type`` can also be set using variable type annotations (see `PEP 526 <https://www.python.org/dev/peps/pep-0526/>`_).
diff --git a/conftest.py b/conftest.py
@@ -1,5 +1,6 @@
 from __future__ import absolute_import, division, print_function
 
+import sys
 import pytest
 
 
@@ -16,3 +17,8 @@ class C(object):
         y = attr()
 
     return C
+
+
+collect_ignore = []
+if sys.version_info[:2] < (3, 6):
+    collect_ignore.append("tests/test_annotations.py")
diff --git a/src/attr/_make.py b/src/attr/_make.py
@@ -125,7 +125,7 @@ def attr(default=NOTHING, validator=None,
         value is converted before being passed to the validator, if any.
     :param metadata: An arbitrary mapping, to be used by third-party
         components.  See :ref:`extending_metadata`.
-    :param type: The type of the attribute.  In python 3.6 or greater, the
+    :param type: The type of the attribute.  In Python 3.6 or greater, the
         preferred method to specify the type is using a variable annotation
         (see PEP-526 ).  This argument is provided for backward compatibility.
         Regardless of the approach used, the type will be stored on
@@ -198,7 +198,7 @@ def _transform_attrs(cls, these):
                    for name, ca
                    in iteritems(these)]
 
-    ann = getattr(cls, '__annotations__', {})
+    ann = getattr(cls, "__annotations__", {})
 
     non_super_attrs = [
         Attribute.from_counting_attr(name=attr_name, ca=ca,
@@ -892,10 +892,10 @@ def from_counting_attr(cls, name, ca, type=None):
         # type holds the annotated value. deal with conflicts:
         if type is None:
             type = ca.type
-        elif ca.type is not None and type is not ca.type:
+        elif ca.type is not None:
             raise ValueError(
-                "Type conflict: annotated type and given type differ: {ann} "
-                "is not {given}.".format(given=ca.type, ann=type)
+                "Type annotation and type argument are both present: "
+                "{ann}, {given}.".format(given=ca.type, ann=type)
             )
         inst_dict = {
             k: getattr(ca, k)

diff --git a/tests/_test_annotations.py → tests/test_annotations.py b/tests/_test_annotations.py → tests/test_annotations.py
@@ -1,5 +1,5 @@
 """
-Tests for python 3 type annotations.
+Tests for PEP-526 type annotations.
 """
 
 from __future__ import absolute_import, division, print_function
@@ -9,6 +9,7 @@
 from attr._make import (
     attr,
     attributes,
+    fields
 )
 
 import typing
@@ -28,20 +29,20 @@ class C(object):
             x: int = attr()
             y = attr(type=str)
             z = attr()
-        assert int is C.__attrs_attrs__[0].type
-        assert str is C.__attrs_attrs__[1].type
-        assert None is C.__attrs_attrs__[2].type
+        assert int is fields(C).x.type
+        assert str is fields(C).y.type
+        assert None is fields(C).z.type
 
     def test_catches_basic_type_conflict(self):
         """
-        Raises ValueError if types conflict.
+        Raises ValueError type is specified both ways.
         """
         with pytest.raises(ValueError) as e:
             @attributes
             class C:
-                x: int = attr(type=str)
-        assert ("Type conflict: annotated type and given type differ: "
-                "<class 'int'> is not <class 'str'>.",) == e.value.args
+                x: int = attr(type=int)
+        assert ("Type annotation and type argument are both present: "
+                "<class 'int'>, <class 'int'>.",) == e.value.args
 
     def test_typing_annotations(self):
         """
@@ -52,16 +53,5 @@ class C(object):
             x: typing.List[int] = attr()
             y = attr(type=typing.Optional[str])
 
-        assert typing.List[int] is C.__attrs_attrs__[0].type
-        assert typing.Optional[str] is C.__attrs_attrs__[1].type
-
-    def test_catches_typing_type_conflict(self):
-        """
-        Raises ValueError if types conflict.
-        """
-        with pytest.raises(ValueError) as e:
-            @attributes
-            class C:
-                x: int = attr(type=typing.List[str])
-        assert ("Type conflict: annotated type and given type differ: "
-                "<class 'int'> is not typing.List[str].",) == e.value.args
+        assert typing.List[int] is fields(C).x.type
+        assert typing.Optional[str] is fields(C).y.type
diff --git a/tests/test_make.py b/tests/test_make.py
@@ -415,9 +415,9 @@ class C(object):
             x = attr(type=int)
             y = attr(type=str)
             z = attr()
-        assert int is C.__attrs_attrs__[0].type
-        assert str is C.__attrs_attrs__[1].type
-        assert None is C.__attrs_attrs__[2].type
+        assert int is fields(C).x.type
+        assert str is fields(C).y.type
+        assert None is fields(C).z.type
 
 
 @attributes

diff --git a/tox.ini b/tox.ini
@@ -18,7 +18,7 @@ commands = coverage run --parallel -m pytest {posargs}
 
 [testenv:py36]
 deps = -rdev-requirements.txt
-commands = coverage run --parallel -m pytest {posargs} tests/_test_annotations.py
+commands = coverage run --parallel -m pytest {posargs}
 
 
 [testenv:flake8]