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

Fix inferred type of is_dataclass(Any) #12517

Merged
merged 2 commits into from
Aug 14, 2024
Merged

Conversation

intgr
Copy link
Contributor

@intgr intgr commented Aug 12, 2024

Fixes #12401 by adding a hack, in the form of overload def is_dataclass(obj: Never) -- because Any is compatible with Never, but no real type is.

Previously is_dataclass(Any) matched the wrong overload rule and inferred incorrect return type.


# HACK: `obj: Never` typing matches if object argument is using `Any` type.
@overload
def is_dataclass(obj: Never) -> TypeIs[DataclassInstance | type[DataclassInstance]]: ... # type: ignore[narrowed-type-not-subtype] # pyright: ignore[reportGeneralTypeIssues]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suppressing mypy error:

stdlib/dataclasses.pyi:219: error: Narrowed type "DataclassInstance | type[DataclassInstance]" is not a subtype of input type "Never" [narrowed-type-not-subtype]

This comment has been minimized.

@intgr intgr force-pushed the fix-is_dataclass-any branch from 74ea978 to b8243c1 Compare August 12, 2024 12:12
Comment on lines +49 to +56
def is_dataclass_object(arg: object) -> None:
if dc.is_dataclass(arg):
assert_type(arg, Union["DataclassInstance", Type["DataclassInstance"]])


def is_dataclass_type(arg: type) -> None:
if dc.is_dataclass(arg):
assert_type(arg, Type["DataclassInstance"])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually these cases are already checked by check_other_isdataclass_overloads below and can be removed.

Suggested change
def is_dataclass_object(arg: object) -> None:
if dc.is_dataclass(arg):
assert_type(arg, Union["DataclassInstance", Type["DataclassInstance"]])
def is_dataclass_type(arg: type) -> None:
if dc.is_dataclass(arg):
assert_type(arg, Type["DataclassInstance"])

@srittau
Copy link
Collaborator

srittau commented Aug 12, 2024

The primer output looks promising. It mostly changes wrong error messages into true positives. (Due to is_dataclass()'s surprising API.)

Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

pydantic (https://github.com/pydantic/pydantic)
+ pydantic/v1/json.py:80: error: Argument 1 to "asdict" has incompatible type "DataclassInstance | type[DataclassInstance]"; expected "DataclassInstance"  [arg-type]
- pydantic/v1/json.py:80: error: No overload variant of "asdict" matches argument type "type[DataclassInstance]"  [call-overload]
- pydantic/v1/json.py:80: note: Possible overload variants:
- pydantic/v1/json.py:80: note:     def asdict(obj: DataclassInstance) -> dict[str, Any]
- pydantic/v1/json.py:80: note:     def [_T] asdict(obj: DataclassInstance, *, dict_factory: Callable[[list[tuple[str, Any]]], _T]) -> _T

pytest (https://github.com/pytest-dev/pytest)
+ src/_pytest/_io/pprint.py:114: error: Unused "type: ignore" comment  [unused-ignore]
+ src/_pytest/_io/pprint.py:116: error: "DataclassInstance" has no attribute "__dataclass_params__"  [attr-defined]
+ src/_pytest/_io/pprint.py:122: error: Unused "type: ignore" comment  [unused-ignore]

core (https://github.com/home-assistant/core)
+ homeassistant/util/frozen_dataclass_compat.py:114: error: Argument 1 to "__new__" of "object" has incompatible type "DataclassInstance | type[DataclassInstance]"; expected "type[DataclassInstance]"  [arg-type]

streamlit (https://github.com/streamlit/streamlit)
+ lib/streamlit/runtime/caching/hashing.py:409:53: error: Argument 1 to "asdict" has incompatible type "Union[DataclassInstance, Type[DataclassInstance]]"; expected "DataclassInstance"  [arg-type]
- lib/streamlit/runtime/caching/hashing.py:409:34: error: No overload variant of "asdict" matches argument type "Type[DataclassInstance]"  [call-overload]
- lib/streamlit/runtime/caching/hashing.py:409:34: note: Possible overload variants:
- lib/streamlit/runtime/caching/hashing.py:409:34: note:     def asdict(obj: DataclassInstance) -> Dict[str, Any]
- lib/streamlit/runtime/caching/hashing.py:409:34: note:     def [_T] asdict(obj: DataclassInstance, *, dict_factory: Callable[[List[Tuple[str, Any]]], _T]) -> _T

koda-validate (https://github.com/keithasaurus/koda-validate)
+ koda_validate/typehints.py:119: error: Argument 1 to "DataclassValidator" has incompatible type "DataclassInstance | type[DataclassInstance]"; expected "type[Any]"  [arg-type]
+ koda_validate/signature.py:81: error: Argument 1 to "DataclassValidator" has incompatible type "DataclassInstance | type[DataclassInstance]"; expected "type[Any]"  [arg-type]
+ koda_validate/signature.py:81: error: Argument 1 to "dataclass_no_coerce" has incompatible type "DataclassInstance | type[DataclassInstance]"; expected "type[DataclassInstance]"  [arg-type]

xarray-dataclasses (https://github.com/astropenguin/xarray-dataclasses)
- xarray_dataclasses/typing.py:300: error: Unused "type: ignore" comment  [unused-ignore]

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I'd like a third opinion.

@max-muoto
Copy link
Contributor

max-muoto commented Aug 12, 2024

Thoughts on adding a TODO to revert things here? Once python/mypy#17579 is fixed. At least making it clear in the stubs this is a MyPy specific issue we're accounting for.

@srittau srittau merged commit 1ace571 into python:main Aug 14, 2024
63 checks passed
max-muoto pushed a commit to max-muoto/typeshed that referenced this pull request Aug 17, 2024
max-muoto pushed a commit to max-muoto/typeshed that referenced this pull request Sep 8, 2024
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

Successfully merging this pull request may close these issues.

is_dataclass(Any) return type incorrect, differs from is_dataclass(object)
3 participants