-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Basic ParamSpec Concatenate and literal support #11847
Conversation
Now this program works: ```py from typing_extensions import ParamSpec from typing import Generic, Callable P = ParamSpec("P") class Z(Generic[P]): def call(self, *args: P.args, **kwargs: P.kwargs) -> None: pass n: Z[[int]] reveal_type(n) # N: Revealed type is "repro.Z[[builtins.int]]" def f(n: Z[P]) -> Z[P]: ... reveal_type(f) # N: Revealed type is "def [P] (n: repro.Z[P`-1]) -> repro.Z[P`-1]" reveal_type(f(n)) # N: Revealed type is "repro.Z[[builtins.int]]" ```
This program: ```py from typing_extensions import ParamSpec, Concatenate from typing import Generic P = ParamSpec("P") class Z(Generic[P]): ... n: Z[[int]] reveal_type(n) def f(n: Z[P]) -> Z[Concatenate[bool, Concatenate[str, P]]]: ... reveal_type(f) reveal_type(f(n)) ``` outputs: ``` repro.py:10: note: Revealed type is "repro.Z[[builtins.int]]" repro.py:14: note: Revealed type is "def [P] (n: repro.Z[P`-1]) -> repro.Z[Concatenate[builtins.bool, builtins.str, P`-1]]" repro.py:15: note: Revealed type is "repro.Z[[builtins.bool, builtins.str, builtins.int]]" ``` Next up: checking inputs match prefixes and why does it only work when cache exists?
It wasn't actually cache...
1e06238
to
ae8ac73
Compare
@@ -506,6 +506,17 @@ def visit_param_spec(self, t: ParamSpecType) -> ProperType: | |||
else: | |||
return self.default(self.s) | |||
|
|||
def visit_parameters(self, t: Parameters) -> ProperType: | |||
# TODO: is this the right variance? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is correct, we need any instance of the return type to be compatible iwth both, which seems to be the intersection of their arguments e.g. meet_types
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, if its possible to test these independently (I assume we can add tests in the semanal section that simply test this, if not thats a problem in itself) the "Add all the visitors" section of the diff should probably be migrated to a separate PR with its own semanal tests.
|
||
# This is a heuristic: it will be checked later anyways but the error | ||
# message may be worse. | ||
with self.set_allow_param_spec_literals(info.has_param_spec_type): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Existing precedent seems to be that anal_type
does its own pushing/popping.
IMO (and we can do this in a separate PR so that this one is a minor tweak) we should do something like having the parameters to anal_type
be some sort of dataclass, and then allow passing mutators to the dataclass to anal_array
at which point it will push/pop the options. This avoids having N things that are pushing/popping to some sort of stack.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its also really confusing how the state set by set_allow_param_spec_literals
compares to the value allow_param_spec
send to self.anal_array
and self.anal_type
. If the distinction is necessary it should be documented somewhere what the difference is.
@A5rocks Great work, thanks! I also hit the same roadblock, and I'm glad to see that you are working on it. Your idea to generalize where
So to do this, we would need to submit PRs to change the CPython source code, the docs, and the PEP. For those to have a chance of being accepted, we would need convincing justifications of the need for the change, not just "because it was there". mypy would still need to reject that syntax for now, but we could guard the rejection by version numbers, in anticipation of removing the restriction in future versions of Python after the PRs are accepted. Truthfully, I'm not really sure why this syntax is needed, other than aesthetics, because EDIT: To be clear, the simplest solution is to stick with the current syntax: |
@A5rocks This close to being ready to be merged! If you can fix the merge conflicts and CI failures, and address feedback in the next 1 week or so, we could probably include this in the next mypy feature release (planned to be out in the first half of April). |
Yep I just somehow misread the syntax definition in PEP 612, fixed.
Looked a bit at it: so the overload resolution is failing for exactly what you predicted: the type ignore. In a nutshell, the overloads are checked through until the first without type errors upon calling it -- this yields I'm not sure a solution would be in scope for this PR (as a proper solution would involve propagating type ignores) EDIT: filed #12454 |
Diff from mypy_primer, showing the effect of this PR on open source code: core (https://github.com/home-assistant/core)
+ homeassistant/helpers/event.py:116: error: Unused "type: ignore" comment
+ homeassistant/helpers/event.py:117: error: Unused "type: ignore" comment
+ homeassistant/components/tplink/entity.py:22: error: Unused "type: ignore" comment
+ homeassistant/components/tplink/entity.py:23: error: Unused "type: ignore" comment
+ homeassistant/components/sonarr/sensor.py:112: error: Unused "type: ignore" comment
+ homeassistant/components/sonarr/sensor.py:113: error: Unused "type: ignore" comment
+ homeassistant/components/evil_genius_labs/util.py:18: error: Unused "type: ignore" comment
+ homeassistant/components/evil_genius_labs/util.py:19: error: Unused "type: ignore" comment
+ homeassistant/components/evil_genius_labs/util.py:27: error: Unused "type: ignore" comment
+ homeassistant/components/zabbix/__init__.py:93: error: Argument 3 has incompatible type "Callable[[datetime], bool]"; expected "Union[HassJob[Optional[Awaitable[None]]], Callable[[datetime], Optional[Awaitable[None]]]]" [arg-type]
+ homeassistant/components/zabbix/__init__.py:93: error: Incompatible return value type (got "bool", expected "Optional[Awaitable[None]]") [return-value]
+ homeassistant/components/plugwise/util.py:18: error: Unused "type: ignore" comment
+ homeassistant/components/plugwise/util.py:19: error: Unused "type: ignore" comment
+ homeassistant/components/roku/helpers.py:32: error: Unused "type: ignore" comment
+ homeassistant/components/roku/helpers.py:33: error: Unused "type: ignore" comment
+ homeassistant/components/webostv/media_player.py:101: error: Unused "type: ignore" comment
+ homeassistant/components/webostv/media_player.py:102: error: Unused "type: ignore" comment
+ homeassistant/components/sonos/helpers.py:38: error: Unused "type: ignore" comment
+ homeassistant/components/sonos/helpers.py:47: error: Unused "type: ignore" comment
+ homeassistant/components/sonos/helpers.py:55: error: Unused "type: ignore" comment
+ homeassistant/components/sonos/helpers.py:61: error: Unused "type: ignore" comment
+ homeassistant/components/sonos/helpers.py:62: error: Unused "type: ignore" comment
+ homeassistant/components/sonos/media.py:138: error: Argument 1 to "int" has incompatible type "Optional[Any]"; expected "Union[str, bytes, SupportsInt, SupportsIndex, SupportsTrunc]" [arg-type]
+ homeassistant/components/vlc_telnet/media_player.py:79: error: Unused "type: ignore" comment
+ homeassistant/components/vlc_telnet/media_player.py:80: error: Unused "type: ignore" comment
+ homeassistant/components/dlna_dmr/media_player.py:82: error: Unused "type: ignore" comment
+ homeassistant/components/dlna_dmr/media_player.py:83: error: Unused "type: ignore" comment
+ homeassistant/components/dlna_dmr/media_player.py:95: error: Unused "type: ignore" comment
|
Thanks for taking the time! I already left a #12454 (comment), but the gist is that I should just fix the argument types itself. So not an issue for this PR. -- The failed windows CI tests seem unrelated. They fail on the master branch, too. |
Windows tests have been fixed in #12477 |
Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
pros: - more readable IMO - more concise - standardization of output cons: - can be easy to miss double backets - I feel like this is a small deviation from normal mypy representations... but I have nothing to back that up. Overall, I think it's a good thing to try at least.
Diff from mypy_primer, showing the effect of this PR on open source code: core (https://github.com/home-assistant/core)
+ homeassistant/helpers/event.py:116: error: Unused "type: ignore" comment
+ homeassistant/helpers/event.py:117: error: Unused "type: ignore" comment
+ homeassistant/components/tplink/entity.py:22: error: Unused "type: ignore" comment
+ homeassistant/components/tplink/entity.py:23: error: Unused "type: ignore" comment
+ homeassistant/components/sonarr/sensor.py:112: error: Unused "type: ignore" comment
+ homeassistant/components/sonarr/sensor.py:113: error: Unused "type: ignore" comment
+ homeassistant/components/evil_genius_labs/util.py:18: error: Unused "type: ignore" comment
+ homeassistant/components/evil_genius_labs/util.py:19: error: Unused "type: ignore" comment
+ homeassistant/components/evil_genius_labs/util.py:27: error: Unused "type: ignore" comment
+ homeassistant/components/zabbix/__init__.py:93: error: Argument 3 has incompatible type "Callable[[datetime], bool]"; expected "Union[HassJob[Optional[Awaitable[None]]], Callable[[datetime], Optional[Awaitable[None]]]]" [arg-type]
+ homeassistant/components/zabbix/__init__.py:93: error: Incompatible return value type (got "bool", expected "Optional[Awaitable[None]]") [return-value]
+ homeassistant/components/plugwise/util.py:18: error: Unused "type: ignore" comment
+ homeassistant/components/plugwise/util.py:19: error: Unused "type: ignore" comment
+ homeassistant/components/roku/helpers.py:32: error: Unused "type: ignore" comment
+ homeassistant/components/roku/helpers.py:33: error: Unused "type: ignore" comment
+ homeassistant/components/webostv/media_player.py:101: error: Unused "type: ignore" comment
+ homeassistant/components/webostv/media_player.py:102: error: Unused "type: ignore" comment
+ homeassistant/components/sonos/helpers.py:38: error: Unused "type: ignore" comment
+ homeassistant/components/sonos/helpers.py:47: error: Unused "type: ignore" comment
+ homeassistant/components/sonos/helpers.py:55: error: Unused "type: ignore" comment
+ homeassistant/components/sonos/helpers.py:61: error: Unused "type: ignore" comment
+ homeassistant/components/sonos/helpers.py:62: error: Unused "type: ignore" comment
+ homeassistant/components/sonos/media.py:138: error: Argument 1 to "int" has incompatible type "Optional[Any]"; expected "Union[str, bytes, SupportsInt, SupportsIndex, SupportsTrunc]" [arg-type]
+ homeassistant/components/vlc_telnet/media_player.py:79: error: Unused "type: ignore" comment
+ homeassistant/components/vlc_telnet/media_player.py:80: error: Unused "type: ignore" comment
+ homeassistant/components/dlna_dmr/media_player.py:82: error: Unused "type: ignore" comment
+ homeassistant/components/dlna_dmr/media_player.py:83: error: Unused "type: ignore" comment
+ homeassistant/components/dlna_dmr/media_player.py:95: error: Unused "type: ignore" comment
|
1 similar comment
Diff from mypy_primer, showing the effect of this PR on open source code: core (https://github.com/home-assistant/core)
+ homeassistant/helpers/event.py:116: error: Unused "type: ignore" comment
+ homeassistant/helpers/event.py:117: error: Unused "type: ignore" comment
+ homeassistant/components/tplink/entity.py:22: error: Unused "type: ignore" comment
+ homeassistant/components/tplink/entity.py:23: error: Unused "type: ignore" comment
+ homeassistant/components/sonarr/sensor.py:112: error: Unused "type: ignore" comment
+ homeassistant/components/sonarr/sensor.py:113: error: Unused "type: ignore" comment
+ homeassistant/components/evil_genius_labs/util.py:18: error: Unused "type: ignore" comment
+ homeassistant/components/evil_genius_labs/util.py:19: error: Unused "type: ignore" comment
+ homeassistant/components/evil_genius_labs/util.py:27: error: Unused "type: ignore" comment
+ homeassistant/components/zabbix/__init__.py:93: error: Argument 3 has incompatible type "Callable[[datetime], bool]"; expected "Union[HassJob[Optional[Awaitable[None]]], Callable[[datetime], Optional[Awaitable[None]]]]" [arg-type]
+ homeassistant/components/zabbix/__init__.py:93: error: Incompatible return value type (got "bool", expected "Optional[Awaitable[None]]") [return-value]
+ homeassistant/components/plugwise/util.py:18: error: Unused "type: ignore" comment
+ homeassistant/components/plugwise/util.py:19: error: Unused "type: ignore" comment
+ homeassistant/components/roku/helpers.py:32: error: Unused "type: ignore" comment
+ homeassistant/components/roku/helpers.py:33: error: Unused "type: ignore" comment
+ homeassistant/components/webostv/media_player.py:101: error: Unused "type: ignore" comment
+ homeassistant/components/webostv/media_player.py:102: error: Unused "type: ignore" comment
+ homeassistant/components/sonos/helpers.py:38: error: Unused "type: ignore" comment
+ homeassistant/components/sonos/helpers.py:47: error: Unused "type: ignore" comment
+ homeassistant/components/sonos/helpers.py:55: error: Unused "type: ignore" comment
+ homeassistant/components/sonos/helpers.py:61: error: Unused "type: ignore" comment
+ homeassistant/components/sonos/helpers.py:62: error: Unused "type: ignore" comment
+ homeassistant/components/sonos/media.py:138: error: Argument 1 to "int" has incompatible type "Optional[Any]"; expected "Union[str, bytes, SupportsInt, SupportsIndex, SupportsTrunc]" [arg-type]
+ homeassistant/components/vlc_telnet/media_player.py:79: error: Unused "type: ignore" comment
+ homeassistant/components/vlc_telnet/media_player.py:80: error: Unused "type: ignore" comment
+ homeassistant/components/dlna_dmr/media_player.py:82: error: Unused "type: ignore" comment
+ homeassistant/components/dlna_dmr/media_player.py:83: error: Unused "type: ignore" comment
+ homeassistant/components/dlna_dmr/media_player.py:95: error: Unused "type: ignore" comment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good now! The core functionality looks solid, and it will be great to have this in the next release. Remaining corner cases and TODOs can be worked on in separate PRs.
Thank you for working on this -- this is the biggest remaining 3.10 feature that mypy has been missing.
Thanks! FYI, #12096 should be checked out (I put similar code in the |
Just want to say that I'm really excited to see this get merged — thanks for all your work on this @A5rocks! |
Description
I was planning on adding a ParamSpec-based interface to something I'm making but... it turns out I need
Concatenate
andZ[[int]]
syntax.Closes #11833
Closes #12276 (needs test)
Closes #12257 (needs test; not sure how to test cache issues)
Refs #8645
External ref python/typeshed#4827
This PR adds a new
Parameters
proper type to represent parameters (more later in this description), along with adding aConcatenate
operator.Test Plan
I copied all (seemingly relevant) tests from PEP 612. I plan on copying test cases from issues and thinking of a few of my own later.
Notes
(not a real section but seemed important enough)
Concatenate
support does not use aProperType
. That would be too much new code for an operator, in my opinion. Thus, it prepends to aParamSpecType
's.prefix
.CallableType
to take in aparameters: ParamSpecType | Parameters
-- this would simplify some code, and it fits better in your head. However, this would be a breaking change, so I did not do this (along with the fact that making basic changes in this direction led to 300 type errors when self-checking).-Concatenate
was super unspecified by the PEP in my opinion. I ended up making it work whereverParamSpec
could be (or so I think)... Which meansConcatenate[int, Concatenate[str, P]]
works!