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

Export DataclassInstance protocol from _typeshed #9676

Merged
merged 2 commits into from
Feb 4, 2023

Conversation

cdce8p
Copy link
Contributor

@cdce8p cdce8p commented Feb 4, 2023

Add DataclassInstance protocol to _typeshed. Especially when dealing with mixin classes, it's quite common to have to use it. Moving it to _typeshed means users can import it from there without having to reimplement it themselves.

Followup to #9362 (comment)
/CC @AlexWaygood

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Any reason not to just put it in _typeshed/__init__.pyi? It feels a bit small to go in its own file. We can always move it to a dataclasses-specific file later if we add more dataclasses-specific protocols or aliases.

Also: I'd like to add a comment noting that the protocol is quasi-experimental, and linking to the pyright issue. The type won't work as intended for pyright users unless and until the pyright issue is reconsidered (or we think of another way of doing this in the stubs).

@github-actions

This comment has been minimized.

@AlexWaygood AlexWaygood changed the title Export DataclassInstance protocol Export DataclassInstance protocol from _typeshed Feb 4, 2023
@AlexWaygood
Copy link
Member

Thanks!

@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2023

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

porcupine (https://github.com/Akuli/porcupine)
- porcupine/utils.py:408: note:     def asdict(obj: _DataclassInstance) -> Dict[str, Any]
+ porcupine/utils.py:408: note:     def asdict(obj: DataclassInstance) -> Dict[str, Any]
- porcupine/utils.py:408: note:     def [_T] asdict(obj: _DataclassInstance, *, dict_factory: Callable[[List[Tuple[str, Any]]], _T]) -> _T
+ porcupine/utils.py:408: note:     def [_T] asdict(obj: DataclassInstance, *, dict_factory: Callable[[List[Tuple[str, Any]]], _T]) -> _T

hydra-zen (https://github.com/mit-ll-responsible-ai/hydra-zen)
- src/hydra_zen/structured_configs/_implementations.py:1862: error: Incompatible types in assignment (expression has type "Union[Type[_DataclassInstance], Type[Any], Callable[P, R], Callable[..., Any], DataClass_]", variable has type overloaded function)  [assignment]
+ src/hydra_zen/structured_configs/_implementations.py:1862: error: Incompatible types in assignment (expression has type "Union[Type[DataclassInstance], Type[Any], Callable[P, R], Callable[..., Any], DataClass_]", variable has type overloaded function)  [assignment]

core (https://github.com/home-assistant/core)
- homeassistant/components/p1_monitor/diagnostics.py:45: error: Argument 1 to "asdict" has incompatible type "Optional[Any]"; expected "_DataclassInstance"  [arg-type]
+ homeassistant/components/p1_monitor/diagnostics.py:45: error: Argument 1 to "asdict" has incompatible type "Optional[Any]"; expected "DataclassInstance"  [arg-type]
- homeassistant/components/zwave_js/discovery.py:78: note:     def asdict(obj: _DataclassInstance) -> Dict[str, Any]
+ homeassistant/components/zwave_js/discovery.py:78: note:     def asdict(obj: DataclassInstance) -> Dict[str, Any]
- homeassistant/components/zwave_js/discovery.py:78: note:     def [_T] asdict(obj: _DataclassInstance, *, dict_factory: Callable[[List[Tuple[str, Any]]], _T]) -> _T
+ homeassistant/components/zwave_js/discovery.py:78: note:     def [_T] asdict(obj: DataclassInstance, *, dict_factory: Callable[[List[Tuple[str, Any]]], _T]) -> _T

@AlexWaygood AlexWaygood merged commit 88a761e into python:main Feb 4, 2023
@cdce8p cdce8p deleted the export-dataclass-instance branch February 4, 2023 16:01
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.

2 participants