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

Type checking fails when registering a structure hook on a Union #105

Closed
tdsmith opened this issue Oct 31, 2020 · 4 comments
Closed

Type checking fails when registering a structure hook on a Union #105

tdsmith opened this issue Oct 31, 2020 · 4 comments

Comments

@tdsmith
Copy link
Contributor

tdsmith commented Oct 31, 2020

  • cattrs version: 1.1.1
  • Python version: 3.8.6, mypy 0.790
  • Operating System: macOS 10.15.7

Description

After upgrading from cattrs 1.0, mypy has started rejecting Union types as the type argument to Converter.register_structure_hook.

I think the docs at https://cattrs.readthedocs.io/en/latest/structuring.html#manual-disambiguation say that this is the supported way to structure a union type. It executes fine, but mypy rejects it.

What I Did

from typing import Any, ClassVar, List, Literal, Union, Type

import attr
import cattr

@attr.s(auto_attribs=True)
class Foo:
    kind: ClassVar[Literal["foo"]] = "foo"
    value: int

@attr.s(auto_attribs=True)
class Bar:
    kind: ClassVar[Literal["bar"]] = "bar"
    value: int

FooBar = Union[Foo, Bar]

def discriminate_foobar(value: Any, _klass: Type) -> FooBar:
    kind, int_value = value["kind"], value["value"]
    return Foo(int_value) if kind == "foo" else Bar(int_value)

cattr.register_structure_hook(FooBar, discriminate_foobar)

print(cattr.structure([{"kind": "foo", "value": 3}, {"kind": "bar", "value": 42}], List[FooBar]))
$ python demo2.py
[Foo(value=3), Bar(value=42)]

$ mypy demo2.py
demo2.py:22: error: Argument 1 has incompatible type "object"; expected "Type[Any]"
@Tinche
Copy link
Member

Tinche commented Oct 31, 2020

Looks like a mypy bug to me, feel free to go over to their issue tracker and log this.

For example,

>>> reveal_type(Union[int, float])
note: Revealed type is 'builtins.object'

@tdsmith
Copy link
Contributor Author

tdsmith commented Nov 2, 2020

I went over to the python/typing gitter and got some feedback from oremanj that this is correct. Unions describe types but don't meet the type contract; they e.g. don't work with isinstance() or issubclass() and they're not notionally instantiable.

We also can't express "a Type or a Union of Types" directly; you can't construct a type hint that accepts any old Union because the type of a Union[...] isn't a Union, it's just object.

Do you buy that? I could go ahead and open an issue against mypy to collect some more feedback, especially on the second point -- my guess is that mypy will be reluctant to make a change but we can explain the use case.

@Tinche
Copy link
Member

Tinche commented Nov 2, 2020

Yeah, I buy that a subset of the type system doesn't fulfil the Type[T] contract. Looks like a Sequence[int] isn't a valid Type[T] either.

This means we can't annotate this method properly, though. It would be nice to have a type to express a valid type hint though. In any case, looks like this is a missing feature instead of a bug in Mypy. I'll take a PR changing this type annotation to Any :)

If you feel like it, feel free to make an issue over at Mypy explaining the situation.

@Tinche
Copy link
Member

Tinche commented Nov 19, 2020

Fixed on master.

@Tinche Tinche closed this as completed Nov 19, 2020
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

No branches or pull requests

2 participants