Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make_class(): Populate the __annotations__ dict #1271

Closed
sscherfke opened this issue Mar 19, 2024 · 3 comments · Fixed by #1285
Closed

make_class(): Populate the __annotations__ dict #1271

sscherfke opened this issue Mar 19, 2024 · 3 comments · Fixed by #1285

Comments

@sscherfke
Copy link
Contributor

make_class() currently doesn't touch the __annotations__ dict when it creates a new class.

This can become a problem when a class is created dynamically with unresolved types. And when __annotations__ is empty, resolve_type() on the generated class will do nothing.

Here is a use case: https://gitlab.com/sscherfke/typed-settings/-/issues/54

There's not much required to fix this:

diff --git a/src/attr/_make.py b/src/attr/_make.py
index 0114582..bbdfc15 100644
--- a/src/attr/_make.py
+++ b/src/attr/_make.py
@@ -3058,7 +3058,12 @@ def make_class(
         True,
     )
 
-    return _attrs(these=cls_dict, **attributes_arguments)(type_)
+    cls = _attrs(these=cls_dict, **attributes_arguments)(type_)
+    # Only add type annotations now or "_attrs()" will complain:
+    cls.__annotations__ = {
+        k: v.type for k, v in cls_dict.items() if v.type is not None
+    }
+    return cls
 
 
 # These are required by within this module so we define them here and merely
diff --git a/tests/test_make.py b/tests/test_make.py
index 736ab8c..e89ee87 100644
--- a/tests/test_make.py
+++ b/tests/test_make.py
@@ -1164,6 +1164,32 @@ class TestMakeClass:
 
         attr.make_class("test", {"id": attr.ib(type=str)}, (MyParent[int],))
 
+    def test_annotations(self):
+        """
+        make_class fills the __annotations__ dict for attributes with a known
+        type.
+        """
+        a = attr.ib(type=bool)
+        b = attr.ib(type=None)  # Won't be added to ann. b/c of unfavorable default
+        c = attr.ib()
+
+        C = attr.make_class("C", {"a": a, "b": b, "c": c})
+        C = attr.resolve_types(C)
+
+        assert C.__annotations__ == {"a": bool}
+
+    def test_annotations_resolve(self):
+        """
+        resolve_types() resolves the annotations added by make_class().
+        """
+        a = attr.ib(type="bool")
+
+        C = attr.make_class("C", {"a": a})
+        C = attr.resolve_types(C)
+
+        assert attr.fields(C).a.type is bool
+        assert C.__annotations__ == {"a": "bool"}
+
 
 class TestFields:
     """

The only problem with this is the unfortunate default of field(type=None), b/c we cannot differentiate between an explicit None type an no type. I’m afraid though, that changing the default of type= to a sentinel object would be a braking change.

@sscherfke
Copy link
Contributor Author

Solved it in Typed Settings, so fixing this is not super important. The __annotations__ dict being available for dynamic classes would be nice nonetheless, though.

@hynek
Copy link
Member

hynek commented May 8, 2024

Any reason you dumped a diff instead of a PR? 😅

@sscherfke
Copy link
Contributor Author

It's just a draft for helping you decide whether you like this change or not. If you like it, I will prepare a PR. :-)

github-merge-queue bot pushed a commit that referenced this issue Jul 13, 2024
* make_class(): Add "__annotations_" to generated class

Fixes: #1271

* Fix PR#

* Flip

---------

Co-authored-by: Hynek Schlawack <hs@ox.cx>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants