Possibly wrong type annotations in register_structure_hook_factory

[sscherfke]

• cattrs version: 22.1.0
• Python version: 3.10
• Operating System: macOS

Description

It seems like the type annotations for Converter.register_structue_hook_factory() are not correct.

They suggest a function signature that, when applied, leads to errors in my tests.

What I Did

My code:

def register_attrs_hook_factory(converter: Converter) -> None:
    """
    Register a hook factory that allows using instances of attrs classes where
    cattrs would normally expect a dictionary.

    These instances are then returned as-is and without further processing.
    """

    def allow_attrs_instances(typ: Any) -> Callable[[Any, Any], Any]:
        # According to mypy, this should be:
        # def structure_attrs(val: Any) -> Any:
        def structure_attrs(val: Any, _: Any) -> Any: 
            if isinstance(val, typ):
                return val
            return converter.structure_attrs_fromdict(val, typ)

        return structure_attrs

    converter.register_structure_hook_factory(has, allow_attrs_instances)  # mypy complains here

Mypy complains when I do def structure_attrs(val: Any, _: Any) -> Any:

src/typed_settings/converters.py:77: error: Argument 2 to "register_structure_hook_factory" of "Converter" 
has incompatible type "Callable[[Any], Callable[[Any, Any], Any]]"; 
expected "Callable[[Any], Callable[[Any], Any]]"  [arg-type]

But pytest complains when I do def structure_attrs(val: Any) -> Any:

typed_settings.exceptions.InvalidValueError: structure_attrs() takes 1 positional argument but 2 were given

IMHO, pytest has the stronger arguments. ;-)

[Tinche]

I think you might be right, actually.

Fact of the matter is, the insides of cattrs aren't checked with Mypy since Mypy is far from being able to handle what we need at time this. So the type hints are essentially just inline type stubs. Hence the preponderence of Any.

[Tinche]

@sscherfke I applied a fix to main, check if it's good for you now?

[sscherfke]

I will try it. There might be other, similar places at well. I’ll check it.

[Tinche]

Bring em forward and we'll iron them out.

[sscherfke]

Looks like I need to create a PR .. 😬

[sscherfke]

Maybe this?

diff --git a/src/cattrs/_compat.py b/src/cattrs/_compat.py
index c8e77cd..6f324cb 100644
--- a/src/cattrs/_compat.py
+++ b/src/cattrs/_compat.py
@@ -163,17 +163,17 @@ if is_py37 or is_py38:
             or (type.__origin__ in (Tuple, tuple) and type.__args__[1] is ...)
         )
 
-    def is_mutable_set(type):
+    def is_mutable_set(type: Any) -> bool:
         return type is set or (
             type.__class__ is _GenericAlias and issubclass(type.__origin__, MutableSet)
         )
 
-    def is_frozenset(type):
+    def is_frozenset(type: Any) -> bool:
         return type is frozenset or (
             type.__class__ is _GenericAlias and issubclass(type.__origin__, FrozenSet)
         )
 
-    def is_mapping(type):
+    def is_mapping(type: Any) -> bool:
         return type in (TypingMapping, dict) or (
             type.__class__ is _GenericAlias
             and issubclass(type.__origin__, TypingMapping)
@@ -192,7 +192,7 @@ if is_py37 or is_py38:
     def is_bare(type):
         return getattr(type, "__args__", None) in bare_generic_args
 
-    def is_counter(type):
+    def is_counter(type: Any) -> bool:
         return (
             type in (Counter, ColCounter)
             or getattr(type, "__origin__", None) is ColCounter
@@ -201,15 +201,15 @@ if is_py37 or is_py38:
     if is_py38:
         from typing import Literal
 
-        def is_literal(type) -> bool:
+        def is_literal(type: Any) -> bool:
             return type.__class__ is _GenericAlias and type.__origin__ is Literal
 
     else:
         # No literals in 3.7.
-        def is_literal(_) -> bool:
+        def is_literal(_: Any) -> bool:
             return False
 
-    def is_generic(obj):
+    def is_generic(obj: Any) -> bool:
         return isinstance(obj, _GenericAlias)
 
     def copy_with(type, args):
@@ -240,12 +240,12 @@ else:
         # Not present on 3.9.0, so we try carefully.
         from typing import _LiteralGenericAlias
 
-        def is_literal(type) -> bool:
+        def is_literal(type: Any) -> bool:
             return type.__class__ is _LiteralGenericAlias
 
     except ImportError:
 
-        def is_literal(_) -> bool:
+        def is_literal(_: Any) -> bool:
             return False
 
     Set = AbcSet
@@ -329,7 +329,7 @@ else:
             or (origin is tuple and type.__args__[1] is ...)
         )
 
-    def is_mutable_set(type):
+    def is_mutable_set(type: Any) -> bool:
         return (
             type in (TypingSet, TypingMutableSet, set)
             or (
@@ -339,7 +339,7 @@ else:
             or (getattr(type, "__origin__", None) in (set, AbcMutableSet, AbcSet))
         )
 
-    def is_frozenset(type):
+    def is_frozenset(type: Any) -> bool:
         return (
             type in (FrozenSet, frozenset)
             or (
@@ -349,12 +349,12 @@ else:
             or (getattr(type, "__origin__", None) is frozenset)
         )
 
-    def is_bare(type):
+    def is_bare(type: Any) -> bool:
         return isinstance(type, _SpecialGenericAlias) or (
             not hasattr(type, "__origin__") and not hasattr(type, "__args__")
         )
 
-    def is_mapping(type):
+    def is_mapping(type: Any) -> bool:
         return (
             type in (TypingMapping, Dict, TypingMutableMapping, dict, AbcMutableMapping)
             or (
@@ -368,13 +368,13 @@ else:
             or issubclass(type, dict)
         )
 
-    def is_counter(type):
+    def is_counter(type: Any) -> bool:
         return (
             type in (Counter, TypingCounter)
             or getattr(type, "__origin__", None) is Counter
         )
 
-    def is_generic(obj):
+    def is_generic(obj: Any) -> bool:
         return isinstance(obj, _GenericAlias) or isinstance(obj, GenericAlias)
 
     def copy_with(type, args):
@@ -385,5 +385,5 @@ else:
         return type.__origin__[args]
 
 
-def is_generic_attrs(type):
+def is_generic_attrs(type: Any) -> bool:
     return is_generic(type) and has(type.__origin__)
diff --git a/src/cattrs/converters.py b/src/cattrs/converters.py
index 6dd4d3e..6cc40ac 100644
--- a/src/cattrs/converters.py
+++ b/src/cattrs/converters.py
@@ -55,6 +55,12 @@ NoneType = type(None)
 T = TypeVar("T")
 V = TypeVar("V")
 
+PredicateFn = Callable[[Type[T]], bool]
+StructureFn = Callable[[Any, Type[T]], int]
+StructureFnFactory = Callable[[Type[T]], StructureFn]
+UnstructureFn = Callable[[T], Any]
+UnstructureFnFactory = Callable[[Type[T]], UnstructureFn]
+
 
 class UnstructureStrategy(Enum):
     """`attrs` classes unstructuring strategies."""
@@ -182,7 +188,7 @@ class BaseConverter:
         self._dict_factory = dict_factory
 
         # Unions are instances now, not classes. We use different registries.
-        self._union_struct_registry: Dict[Any, Callable[[Any, Type[T]], T]] = {}
+        self._union_struct_registry: Dict[Any, StructureFn] = {}
 
     def unstructure(self, obj: Any, unstructure_as: Any = None) -> Any:
         return self._unstructure_func.dispatch(
@@ -198,7 +204,7 @@ class BaseConverter:
             else UnstructureStrategy.AS_TUPLE
         )
 
-    def register_unstructure_hook(self, cls: Any, func: Callable[[Any], Any]) -> None:
+    def register_unstructure_hook(self, cls: Any, func: UnstructureFn) -> None:
         """Register a class-to-primitive converter function for a class.
 
         The converter function should take an instance of the class and return
@@ -212,7 +218,7 @@ class BaseConverter:
             self._unstructure_func.register_cls_list([(cls, func)])
 
     def register_unstructure_hook_func(
-        self, check_func: Callable[[Any], bool], func: Callable[[T], Any]
+        self, check_func: PredicateFn, func: UnstructureFn
     ):
         """Register a class-to-primitive converter function for a class, using
         a function to check if it's a match.
@@ -220,9 +226,7 @@ class BaseConverter:
         self._unstructure_func.register_func_list([(check_func, func)])
 
     def register_unstructure_hook_factory(
-        self,
-        predicate: Callable[[Any], bool],
-        factory: Callable[[Any], Callable[[Any], Any]],
+        self, predicate: PredicateFn, factory: UnstructureFnFactory
     ) -> None:
         """
         Register a hook factory for a given predicate.
@@ -235,7 +239,7 @@ class BaseConverter:
         """
         self._unstructure_func.register_func_list([(predicate, factory, True)])
 
-    def register_structure_hook(self, cl: Any, func: Callable[[Any, Type[T]], T]):
+    def register_structure_hook(self, cl: Any, func: StructureFn):
         """Register a primitive-to-class converter function for a type.
 
         The converter function should take two arguments:
@@ -253,18 +257,14 @@ class BaseConverter:
         else:
             self._structure_func.register_cls_list([(cl, func)])
 
-    def register_structure_hook_func(
-        self, check_func: Callable[[Type[T]], bool], func: Callable[[Any, Type[T]], T]
-    ):
+    def register_structure_hook_func(self, check_func: PredicateFn, func: StructureFn):
         """Register a class-to-primitive converter function for a class, using
         a function to check if it's a match.
         """
         self._structure_func.register_func_list([(check_func, func)])
 
     def register_structure_hook_factory(
-        self,
-        predicate: Callable[[Any], bool],
-        factory: Callable[[Any], Callable[[Any, Any], Any]],
+        self, predicate: PredicateFn, factory: StructureFnFactory
     ) -> None:
         """
         Register a hook factory for a given predicate.