From 0bdeee985386b685c0f340fe1f4fb10e7f19861b Mon Sep 17 00:00:00 2001
From: Alex Ford <alex.ford@abcellera.com>
Date: Fri, 25 Dec 2020 11:42:33 -0800
Subject: [PATCH 1/3] Clarify next-gen auto_attribs inference rules

The next-gen auto_attribs api documentation does not clearly describe
cases where (a) only a subset of attributes are annotated and (b)
`field` definitions are provided for a subset of fields. Update
docstring to reflect current, desirable, behavior.

Add tests to clarify `.define` behavior focused on fully-annotated
classes with partially-defined fields, which is commonly used to add
non-default behavior to a subset of a classes fields. For example:

```python

@attr.define
class NewSchool:
    x: int
    y: list = attr.field()

    @y.validator
    def _validate_y(self, attribute, value):
        if value < 0:
            raise ValueError("y must be positive")

```

The previous docstring *could* be read to imply that:

* The new-school API will not infer auto_attribs if there are any
  unannotated attributes.
* The new-school API will not infer auto_attribs if *any* attr.ib are
  defined, even if those attr.ibs are type annotated.
---
 src/attr/_next_gen.py  |  4 ++--
 tests/test_next_gen.py | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/src/attr/_next_gen.py b/src/attr/_next_gen.py
index 2b5565c56..9304e67b3 100644
--- a/src/attr/_next_gen.py
+++ b/src/attr/_next_gen.py
@@ -42,8 +42,8 @@ def define(
     :param Optional[bool] auto_attribs: If set to `True` or `False`, it behaves
        exactly like `attr.s`. If left `None`, `attr.s` will try to guess:
 
-       1. If all attributes are annotated and no `attr.ib` is found, it assumes
-          *auto_attribs=True*.
+       1. If any attributes are annotated and no unannotated `attr.ib` are found,
+          it assumes *auto_attribs=True*.
        2. Otherwise it assumes *auto_attribs=False* and tries to collect
           `attr.ib`\ s.
 
diff --git a/tests/test_next_gen.py b/tests/test_next_gen.py
index 0ebad8d2f..3cb42a377 100644
--- a/tests/test_next_gen.py
+++ b/tests/test_next_gen.py
@@ -112,6 +112,39 @@ class OldSchool2:
 
         assert OldSchool2(1) == OldSchool2(1)
 
+    def test_auto_attribs_detect_fields_and_annotations(self):
+        """
+        define infers auto_attribs=True if fields have type annotations
+        """
+
+        @attr.define
+        class NewSchool:
+            x: int
+            y: list = attr.field(factory=list)
+
+        assert NewSchool(1) == NewSchool(1, [])
+        assert list(attr.fields_dict(NewSchool).keys()) == ["x", "y"]
+
+    def test_auto_attribs_partially_annotated(self):
+        """
+        define infers auto_attribs=True if any type annotations are found
+        """
+
+        @attr.define
+        class NewSchool:
+            x: int
+            y: list
+            z = 10
+
+        # fields are defined for any annotated attributes
+        assert NewSchool(1, []) == NewSchool(1, [])
+        assert list(attr.fields_dict(NewSchool).keys()) == ["x", "y"]
+
+        # while the unannotated attributes are left as class vars
+        assert NewSchool.z == 10
+        assert "z" in NewSchool.__dict__
+
+
     def test_auto_attribs_detect_annotations(self):
         """
         define correctly detects if a class has type annotations.

From 33e60a670928e2bbe94d795ffde8440511581130 Mon Sep 17 00:00:00 2001
From: Alex Ford <alex.ford@abcellera.com>
Date: Fri, 25 Dec 2020 13:34:13 -0800
Subject: [PATCH 2/3] Update test to match PR example

---
 tests/test_next_gen.py | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/tests/test_next_gen.py b/tests/test_next_gen.py
index 3cb42a377..fce01ad45 100644
--- a/tests/test_next_gen.py
+++ b/tests/test_next_gen.py
@@ -120,9 +120,16 @@ def test_auto_attribs_detect_fields_and_annotations(self):
         @attr.define
         class NewSchool:
             x: int
-            y: list = attr.field(factory=list)
+            y: list = attr.field()
 
-        assert NewSchool(1) == NewSchool(1, [])
+            @y.validator
+            def _validate_y(self, attribute, value):
+                if value < 0:
+                    raise ValueError("y must be positive")
+
+        assert NewSchool(1, 1) == NewSchool(1, 1)
+        with pytest.raises(ValueError):
+            NewSchool(1, -1)
         assert list(attr.fields_dict(NewSchool).keys()) == ["x", "y"]
 
     def test_auto_attribs_partially_annotated(self):
@@ -144,7 +151,6 @@ class NewSchool:
         assert NewSchool.z == 10
         assert "z" in NewSchool.__dict__
 
-
     def test_auto_attribs_detect_annotations(self):
         """
         define correctly detects if a class has type annotations.

From f6aca14d14c769728175b1a151067f1ed1aee097 Mon Sep 17 00:00:00 2001
From: Alex Ford <alex.ford@abcellera.com>
Date: Fri, 25 Dec 2020 13:35:40 -0800
Subject: [PATCH 3/3] Fix lint error

---
 src/attr/_next_gen.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/attr/_next_gen.py b/src/attr/_next_gen.py
index 9304e67b3..2990ad257 100644
--- a/src/attr/_next_gen.py
+++ b/src/attr/_next_gen.py
@@ -42,8 +42,8 @@ def define(
     :param Optional[bool] auto_attribs: If set to `True` or `False`, it behaves
        exactly like `attr.s`. If left `None`, `attr.s` will try to guess:
 
-       1. If any attributes are annotated and no unannotated `attr.ib` are found,
-          it assumes *auto_attribs=True*.
+       1. If any attributes are annotated and no unannotated `attr.ib`\ s
+          are found, it assumes *auto_attribs=True*.
        2. Otherwise it assumes *auto_attribs=False* and tries to collect
           `attr.ib`\ s.