python-attrs / cattrs

Composable custom class converters for attrs, dataclasses and friends.
https://catt.rs
MIT License
829 stars 113 forks source link

Spurious Type failure on `register_structure_hook_factory`? #578

Closed stephenprater closed 2 months ago

stephenprater commented 2 months ago

Description

When attempting to use register_structure_hook_factory for my custom structure hook, I get a type error that the decorator expects 1 more positional argument:

@my_converter.register_structure_hook_factory(lambda r: r is MyClass)
def register_review_category_struct(cls: Type[MyClass], conv: cattrs.Converter) -> StructureHook:
    return cattrs.gen.make_dict_structure_fn(
        cls,
        conv,
    )
Screenshot 2024-09-04 at 9 19 21 AM

Using pyright 1.1.379.

The structure factory works in that it creates the correct methods and that my_converter.structure(a_dict, MyClass) - does what it's supposed to but I'm wondering if I'm missing something or if the type error is spurious.

ericbn commented 2 months ago

Also getting an error with mypy. For the following code:

from collections.abc import Callable
from typing import Any

from attr import fields, has
from cattrs import Converter
from cattrs.gen import make_dict_unstructure_fn, override

def _my_converter_unstructure_override_field(f):
    if f.name.startswith("_"):
        return override(omit=True)
    if f.default is None:
        return override(omit_if_default=True)
    return None

@my_converter.register_unstructure_hook_factory(has)
def _my_converter_unstructure_override_class(cl: Any, converter: Converter) -> Callable:
    return make_dict_unstructure_fn(
        cl,
        converter,
        **{
            f.name: v
            for f in fields(cl)
            if (v := _my_converter_unstructure_override_field(f)) is not None
        },
    )

mypy will fail with:

error: Value of type variable "UnstructureHookFactory" of function cannot be "Callable[[Any, Converter], Callable[..., Any]]"  [type-var]
Tinche commented 2 months ago

You folks are right, the type hints need improvement here. This might be more complex than I thought.

Tinche commented 2 months ago

Can you folks try https://github.com/python-attrs/cattrs/tree/tin/fix-type-hints and see if it helps?

ericbn commented 2 months ago

@Tinche, thank you for the prompt response. Using the tin/fix-type-hints branch (resolved to commit dc50023dc32a8191332106361194ff6e31193613) worked for me. No more mypy errors using the same code as before.

EDIT: For a quick reference, it can be installed locally using pip install git+https://github.com/python-attrs/cattrs.git@tin/fix-type-hints

Tinche commented 2 months ago

Yeah, according to my tests this is a big improvement. I'm planning on releasing this as a minor release in a few days.

Tinche commented 2 months ago

Fix released!