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

Accept Ellipsis as last argument for Concatenate. #14656

Closed
randolf-scholz opened this issue Feb 8, 2023 · 14 comments · Fixed by #15905
Closed

Accept Ellipsis as last argument for Concatenate. #14656

randolf-scholz opened this issue Feb 8, 2023 · 14 comments · Fixed by #15905
Labels
bug mypy got something wrong topic-paramspec PEP 612, ParamSpec, Concatenate

Comments

@randolf-scholz
Copy link
Contributor

randolf-scholz commented Feb 8, 2023

Bug Report

from typing import Callable, Concatenate, TypeAlias

intfun: TypeAlias = Callable[Concatenate[int, ...], None]  # ✘ Unexpected "..."  [misc]

https://mypy-play.net/?mypy=master&python=3.11&gist=f4ba9e669549fae3b1a75e02af311db2

Expected Behavior

According to docs https://docs.python.org/3/library/typing.html#typing.Concatenate:

The last parameter to Concatenate must be a ParamSpec or ellipsis (...).

This change deviates from PEP 612 and was introduced in bpo-44791

@randolf-scholz randolf-scholz added the bug mypy got something wrong label Feb 8, 2023
@A5rocks
Copy link
Contributor

A5rocks commented Feb 12, 2023

Hi! This is a good idea, but I think we have to do some work before adding this to mypy.

PEP 1 states:

If changes based on implementation experience and user feedback are made to Standards track PEPs while in the Provisional or (with SC approval) Accepted state, they should be noted in the PEP, such that the PEP accurately describes the implementation at the point where it is marked Final.

Following from this, I think we need to update the PEP. I'm not sure about that "with [steering committee] approval" but PEP 1 is clear that when PEP 621 is put in a Final state, it should accurately reflect implementations (ie allowing ... as the last argument to concatenate).

To update the PEP, it seems we should run it by the author first...


@mrkmndz are you fine with this change? You've probably said something about this before but I can't find it on a cursory look.

@erictraut
Copy link

PEP 612 is final, so it cannot be amended at this point.

Mypy can deviate from the typing standards if its maintainers decide to do so, but if you want this change to be incorporated into the runtime, typing_extensions, and other type checkers, then it would need to be part of a new PEP, or at the very least a discussion in the python/typing discussion forum so all affected parties can join in to the discussion.

I'll point out that there is already a way to express this concept in the Python type system via a callback protocol. The proposal would offer a (slightly) terser way to express the same thing. It saves one line.

class IntFun(Protocol):
    def __call__(self, __x: int, *args: Any, **kwargs: Any) -> None: ...

versus

IntFun: TypeAlias = Callable[Concatenate[int, ...], None]

Normally, the bar is quite high for introducing a redundant mechanism. You would need to demonstrate convincingly that this is a common enough pattern to justify it.

@A5rocks
Copy link
Contributor

A5rocks commented Feb 12, 2023

PEP 612 is final

Currently its status is Accepted; not Final :^)

This is probably an oversight tbf, but it opens the gate for features like this.

but if you want this change to be incorporated into the runtime, typing_extensions, and other type checkers

Additionally, this already holds true at runtime (and stdlib docs) which is part of the way there!

@randolf-scholz
Copy link
Contributor Author

@erictraut That would be a good alternative solution. Currently it is unsupported by mypy.

Mypy does support def __call__(self, *args: Any, **kwargs: Any) (#5876), but it does not work with partial signatures. (#5876 (comment)).

Maybe, in view of the addition of Concatenate, one should reopen #6077?

@erictraut
Copy link

@randolf-scholz, mypy is correct in emitting an error in your example because function foo does not accept *args and **kwargs, as indicated by the protocol. I didn't realize that you were expecting that to work.

So you are looking for a way in the type system to represent a function that accepts a single positional argument that is type int followed by some specific number of positional or keyword arguments (and possibly none)? Why is that concept useful in a type system? It's not possible to use such a value in a type-safe manner because there's no way to know what arguments such a call safely accepts.

If this is really what you want, you could use the following:

P = ParamSpec("P")
_IntFun: TypeAlias = Callable[Concatenate[int, P], None]
IntFun: TypeAlias = _IntFun[...]

Mypy currently emits an error here, but it's legal according to PEP 612, so it appears to just be a bug.

@randolf-scholz
Copy link
Contributor Author

So you are looking for a way in the type system to represent a function that accepts a single positional argument that is type int followed by some specific number of positional or keyword arguments (and possibly none)? Why is that concept useful in a type system?

The point is to be able to represent partial signatures. This has been requested multiple times, so I think I am not doing something super arcane here. (#5876 (comment), #6077, #8263, python/typing#696)

A common use-case are decorators that add/remove arguments, as described in https://peps.python.org/pep-0612/#motivation. Another use-case is when the remaining arguments are provided at-runtime from a config-file.

It's not possible to use such a value in a type-safe manner because there's no way to know what arguments such a call safely accepts.

Well, if it is known that the first argument of a Callable must be int, but the remainder of the signature is unknown (including even the number of args!), then the type checker can still verify that whenever the function is called the first argument is int. I think this is well in line with the idea of gradual typing.

@Viicos
Copy link
Contributor

Viicos commented Feb 18, 2023

class IntFun(Protocol):
    def __call__(self, __x: int, *args: Any, **kwargs: Any) -> None: ...

Note that the following definition will not work with the following function:

def test_func(arbitrary_arg: int, *args: str, **kwargs: Any) -> None:
    return None

You will have to specify arbitrary_arg to be a positional-only argument, probably because the first IntFun argument name is ignored thanks to the leading __ (see mypy docs). So the following works:

def test_func(arbitrary_arg: int, /, *args: str, **kwargs: Any) -> None:
    return None

@JelleZijlstra JelleZijlstra added the topic-paramspec PEP 612, ParamSpec, Concatenate label May 29, 2023
@Jacob-Flasheye
Copy link

Sorry for spamming you with comments earlier, it was towards the end of work and I didn't read the issue properly.
Could someone explain what part of PEP 612 forbids this behaviour? I read the pep but can't find any mention that ... cannot be used, just no example that it can be used. I would guess the grammar is what forbids this behaviour, but my understanding is that ... is a valid expression_type and thus should be legal. I am most likely wrong but I don't understand why.

@erictraut
Copy link

@Jacob-Flasheye, it's documented in the section titled Valid use locations. Here's the specific portion:

parameters_expression ::=
  | "..."
  | "[" [ type_expression ("," type_expression)* ] "]"
  | parameter_specification_variable
  | concatenate "["
                   type_expression ("," type_expression)* ","
                   parameter_specification_variable
                "]"

I recommend closing this issue. If someone wants to propose a change to the type system to support ellipsis, the mypy issue tracker isn't the right place for this. It should be discussed in the python/typing discussion forum.

@hauntsaninja hauntsaninja closed this as not planned Won't fix, can't repro, duplicate, stale Aug 14, 2023
@Viicos
Copy link
Contributor

Viicos commented Aug 14, 2023

Should the Python docs be updated then? As they explicitly state (as OP mentioned) that ... is allowed as the last parameter to Concatenate

@hauntsaninja
Copy link
Collaborator

cc @JelleZijlstra do you have context on what the intended state is here?

@AlexWaygood
Copy link
Member

AlexWaygood commented Aug 14, 2023

The change to the CPython runtime behaviour and the typing docs was made in the CPython PR python/cpython#30969. That change was made after a lengthy discussion in the CPython issue python/cpython#88954 between the authors of PEP 612 and the typing-module maintainers at CPython.

A PEP is a historical document that represents the consensus of the community at a fixed point in time; where a PEP and the "living documentation" at CPython contradict each other, the living documentation always takes precedence. We don't have a great mechanism right now for tweaking typing specs without writing a whole new PEP, and it's arguable whether or not the discussion in python/cpython#88954 was visible enough to the community as a whole. As a practical matter, however, I think we should view the spec as having been amended following the discussion in python/cpython#88954. Mypy should therefore change its behaviour here, as it is not following the amended spec.

@AlexWaygood AlexWaygood reopened this Aug 14, 2023
@erictraut
Copy link

Ah, thanks for the links to the threads. Since that has now been added to the spec, I'll add it to pyright as well.

ilevkivskyi added a commit that referenced this issue Aug 19, 2023
Fixes #14761
Fixes #15318
Fixes #14656
Fixes #13518

I noticed there is a bunch of inconsistencies in `semanal`/`typeanal`
for ParamSpecs, so I decided do a small cleanup. Using this opportunity
I also allow `Concatenate[int, ...]` (with literal Ellipsis), and reduce
verbosity of some errors.

cc @A5rocks
@hauntsaninja
Copy link
Collaborator

@randolf-scholz is the project you're running into this on open source?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-paramspec PEP 612, ParamSpec, Concatenate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants