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

Spurious Type failure on register_structure_hook_factory? #578

Closed
stephenprater opened this issue Sep 4, 2024 · 6 comments · Fixed by #579
Closed

Spurious Type failure on register_structure_hook_factory? #578

stephenprater opened this issue Sep 4, 2024 · 6 comments · Fixed by #579
Labels

Comments

@stephenprater
Copy link

  • cattrs version - 24.0.1
  • Python version - 3.12.4
  • Operating System - OSX 14.6.1

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
Copy link
Contributor

ericbn commented Sep 4, 2024

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
Copy link
Member

Tinche commented Sep 5, 2024

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

@Tinche Tinche added this to the 24.2 milestone Sep 5, 2024
@Tinche Tinche added the bug label Sep 5, 2024
@Tinche
Copy link
Member

Tinche commented Sep 5, 2024

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

@ericbn
Copy link
Contributor

ericbn commented Sep 5, 2024

@Tinche, thank you for the prompt response. Using the tin/fix-type-hints branch (resolved to commit dc50023) 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 Tinche linked a pull request Sep 7, 2024 that will close this issue
@Tinche
Copy link
Member

Tinche commented Sep 9, 2024

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
Copy link
Member

Tinche commented Sep 11, 2024

Fix released!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants