Repository: python-attrs/attrs
Pull request #1099 (2023-02-02 11:52:10)

Flesh out resolve_types
Trying to improve `resolve_types`.

Tinche: @hynek vroom vroom?
hynek: Do we know someone who knows what any of this means and could review?
euresti: Hello.  Happy to review.

So this is backwards incompatible change because before the default was for `include_extras=False` and we've change it to be `True`.  I'm not sure this is a complete deal breaker though as this variable only matters when using `Annotated`.
See https://peps.python.org/pep-0593/#interaction-with-get-type-hints

From the get_type_hints documentation:
 The function recursively replaces all Annotated[T, ...] with T, unless include_extras is set to True (see [Annotated](https://docs.python.org/3/library/typing.html#typing.Annotated) for more information).

So I guess my question is if we're happy with the documentation of "this resolves the types more accurately."
Tinche: You're right in that this is technically backwards incompatible, I made the call with the following assumptions:

* the change is relatively minor (I'm not aware of any use case other than `Annotated` at this time, I think)
* the change will affect very few users (subjective, because no one has opened an issue about not having access to `Annotated` via `resolve_types`)
* the new default is, IMO, more correct so I'd prefer to suffer a little pain now rather than drag the bad default on forever

Happy to change the default if you folks think that's the right call.

As for the docs, happy to be more precise if we think that'll be useful to the readers.
euresti: I wonder if for the docs it could say that the value is passed in to get_type_hints and leave it at that.  Since it's possible the definition of include_extras could change in the future?  (As in maybe it does more than Annotated and now our docs would be out of date with python.). But that's a pretty minor difference all things considered.

As to the default value, I'm of 2 minds. It's nice when you get the latest and greatest by default.  However I'm not sure I like that our default is the opposite of python's.  That being said, our method is not called the same as theirs so it might be ok to reverse the default here.


hynek: Yeah I think I'm fine with this "breakage" since nobody except Tin is using `Annotated` and this seems still like a beta feature. Hiding it behind an option would give us another `auto_attribs=True`.
hynek: Eh sorry, was this ready for merge?
Tinche: Yeah, barring any bikeshedding around the docs.

Patch:
diff --git a/changelog.d/1099.change.md b/changelog.d/1099.change.md
new file mode 100644
index 0000000..a2114ae
--- /dev/null
+++ b/changelog.d/1099.change.md
@@ -0,0 +1,1 @@
+`attrs.resolve_types()` can now pass `include_extras` to `typing.get_type_hints()` on Python 3.9+, and does so by default.
diff --git a/src/attr/__init__.pyi b/src/attr/__init__.pyi
index 1e70b59..003dac4 100644
--- a/src/attr/__init__.pyi
+++ b/src/attr/__init__.pyi
@@ -69,6 +69,7 @@ _ValidatorArgType = Union[_ValidatorType[_T], Sequence[_ValidatorType[_T]]]
 class AttrsInstance(AttrsInstance_, Protocol):
     pass
 
+_A = TypeVar("_A", bound=AttrsInstance)
 # _make --
 
 class _Nothing(enum.Enum):
@@ -488,11 +489,12 @@ def fields(cls: Type[AttrsInstance]) -> Any: ...
 def fields_dict(cls: Type[AttrsInstance]) -> Dict[str, Attribute[Any]]: ...
 def validate(inst: AttrsInstance) -> None: ...
 def resolve_types(
-    cls: _C,
+    cls: _A,
     globalns: Optional[Dict[str, Any]] = ...,
     localns: Optional[Dict[str, Any]] = ...,
     attribs: Optional[List[Attribute[Any]]] = ...,
-) -> _C: ...
+    include_extras: bool = ...,
+) -> _A: ...
 
 # TODO: add support for returning a proper attrs class from the mypy plugin
 # we use Any instead of _CountingAttr so that e.g. `make_class('Foo',
diff --git a/src/attr/_compat.py b/src/attr/_compat.py
index bd44d4f..c3bf5e3 100644
--- a/src/attr/_compat.py
+++ b/src/attr/_compat.py
@@ -13,6 +13,7 @@ from typing import _GenericAlias
 
 
 PYPY = platform.python_implementation() == "PyPy"
+PY_3_9_PLUS = sys.version_info[:2] >= (3, 9)
 PY310 = sys.version_info[:2] >= (3, 10)
 PY_3_12_PLUS = sys.version_info[:2] >= (3, 12)
 
diff --git a/src/attr/_funcs.py b/src/attr/_funcs.py
index 6fa2456..fe11073 100644
--- a/src/attr/_funcs.py
+++ b/src/attr/_funcs.py
@@ -3,7 +3,7 @@
 
 import copy
 
-from ._compat import get_generic_base
+from ._compat import PY_3_9_PLUS, get_generic_base
 from ._make import NOTHING, _obj_setattr, fields
 from .exceptions import AttrsAttributeNotFoundError
 
@@ -379,7 +379,9 @@ def evolve(inst, **changes):
     return cls(**changes)
 
 
-def resolve_types(cls, globalns=None, localns=None, attribs=None):
+def resolve_types(
+    cls, globalns=None, localns=None, attribs=None, include_extras=True
+):
     """
     Resolve any strings and forward annotations in type annotations.
 
@@ -399,6 +401,10 @@ def resolve_types(cls, globalns=None, localns=None, attribs=None):
     :param Optional[list] attribs: List of attribs for the given class.
         This is necessary when calling from inside a ``field_transformer``
         since *cls* is not an *attrs* class yet.
+    :param bool include_extras: Resolve more accurately, if possible.
+        Pass ``include_extras`` to ``typing.get_hints``, if supported by the
+        typing module. On supported Python versions (3.9+), this resolves the
+        types more accurately.
 
     :raise TypeError: If *cls* is not a class.
     :raise attrs.exceptions.NotAnAttrsClassError: If *cls* is not an *attrs*
@@ -411,6 +417,7 @@ def resolve_types(cls, globalns=None, localns=None, attribs=None):
 
     ..  versionadded:: 20.1.0
     ..  versionadded:: 21.1.0 *attribs*
+    ..  versionadded:: 23.1.0 *include_extras*
 
     """
     # Since calling get_type_hints is expensive we cache whether we've
@@ -418,7 +425,12 @@ def resolve_types(cls, globalns=None, localns=None, attribs=None):
     if getattr(cls, "__attrs_types_resolved__", None) != cls:
         import typing
 
-        hints = typing.get_type_hints(cls, globalns=globalns, localns=localns)
+        kwargs = {"globalns": globalns, "localns": localns}
+
+        if PY_3_9_PLUS:
+            kwargs["include_extras"] = include_extras
+
+        hints = typing.get_type_hints(cls, **kwargs)
         for field in fields(cls) if attribs is None else attribs:
             if field.name in hints:
                 # Since fields have been frozen we must work around it.


Test patch:
diff --git a/tests/test_annotations.py b/tests/test_annotations.py
index 0ff68c7..f5ad41d 100644
--- a/tests/test_annotations.py
+++ b/tests/test_annotations.py
@@ -516,6 +516,34 @@ class TestAnnotations:
         assert str is attr.fields(C).y.type
         assert None is attr.fields(C).z.type
 
+    @pytest.mark.skipif(
+        sys.version_info[:2] < (3, 9),
+        reason="Incompatible behavior on older Pythons",
+    )
+    def test_extra_resolve(self):
+        """
+        `get_type_hints` returns extra type hints.
+        """
+        from typing import Annotated
+
+        globals = {"Annotated": Annotated}
+
+        @attr.define
+        class C:
+            x: 'Annotated[float, "test"]'
+
+        attr.resolve_types(C, globals)
+
+        assert attr.fields(C).x.type == Annotated[float, "test"]
+
+        @attr.define
+        class D:
+            x: 'Annotated[float, "test"]'
+
+        attr.resolve_types(D, globals, include_extras=False)
+
+        assert attr.fields(D).x.type == float
+
     def test_resolve_types_auto_attrib(self, slots):
         """
         Types can be resolved even when strings are involved.