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

Pyright fails to recognize valid calls when ParamSpec, Concatenate, and factories are used #3559

Closed
lexicalunit opened this issue Jun 9, 2022 · 7 comments
Labels
as designed Not a bug, working as intended

Comments

@lexicalunit
Copy link

lexicalunit commented Jun 9, 2022

Describe the bug
Contrived code but in the real version this makes sense as we're working with pytest and using the factories-as-fixtures pattern.

See the inline comments on the three calls to bulk_maker() for the pyright errors:

from typing import Any, Callable, List

# For python 3.8:
from typing_extensions import Concatenate, ParamSpec, TypeAlias

# For python 3.10:
# from typing import Concatenate, ParamSpec, TypeAlias


class Company:
    id: int

    def __init__(self, id: int):
        self.id = id
        print(self)

    def __repr__(self) -> str:
        return f"<company {self.id}>"


Factory: TypeAlias = Callable[..., Company]


def make_company(*args: Any, **kwargs: Any) -> Company:
    return Company(*args, **kwargs)


P = ParamSpec("P")
BulkFactory: TypeAlias = Callable[Concatenate[int, P], List[Company]]


def make_n_companies() -> BulkFactory[P]:
    def factory(n: int, *args: P.args, **kwargs: P.kwargs) -> List[Company]:
        return [make_company(*args, **kwargs) for _ in range(n)]

    return factory


bulk_maker = make_n_companies()

print()
bulk_maker(5, 10)  # Expected 1 positional argument

print()
bulk_maker(5, id=10)  # No parameter named "id"

print()
bulk_maker(5)  # Arguments for ParamSpec "P@make_n_companies" are missing

The first two calls to bulk_maker() are fine. The last one is incorrect and will fail at runtime:

± python example.py

<company 10>
<company 10>
<company 10>
<company 10>
<company 10>

<company 10>
<company 10>
<company 10>
<company 10>
<company 10>

Traceback (most recent call last):
  File "foo.py", line 47, in <module>
    bulk_maker(5)  # Arguments for ParamSpec "P@make_n_companies" are missing
  File "foo.py", line 33, in factory
    return [make_company(*args, **kwargs) for _ in range(n)]
  File "foo.py", line 33, in <listcomp>
    return [make_company(*args, **kwargs) for _ in range(n)]
  File "foo.py", line 24, in make_company
    return Company(*args, **kwargs)
TypeError: __init__() missing 1 required positional argument: 'id'

Expected behavior
There should not be any pyright errors for the first two calls to bulk_maker().

VS Code extension or command-line

  • Are you running pyright as a VS Code extension or a command-line tool? Both
  • Which version? pyright 1.1.253 and **pylance v2022.6.10`
  • Error is seen when using python 3.8 and 3.10. No difference.
@erictraut
Copy link
Collaborator

This is an incorrect usage of ParamSpec. The problem is P is scoped to the function make_n_companies, but P appears only in the return type annotation, so there's no way for P to be solved in the context of a call to make_n_companies.

If your intent is to support any callable whose first parameter is an int, then you should parameterize the ParamSpec with ..., as in:

def make_n_companies() -> BulkFactory[...]:
   ...

The ... is the equivalent of Any for a ParamSpec.

@erictraut erictraut added the as designed Not a bug, working as intended label Jun 9, 2022
@lexicalunit
Copy link
Author

def make_n_companies() -> BulkFactory[...]:
   ...

This doesn't actually work at all though. If I make this change I get:

± python3.10 example.py
Traceback (most recent call last):
  File "exmaple.py", line 32, in <module>
    def make_n_companies() -> BulkFactory[...]:
  File ".../Versions/3.10/lib/python3.10/typing.py", line 312, in inner
    return func(*args, **kwds)
  File ".../Versions/3.10/lib/python3.10/typing.py", line 1075, in __getitem__
    arg = arg[subargs]
  File ".../Versions/3.10/lib/python3.10/typing.py", line 312, in inner
    return func(*args, **kwds)
  File ".../Versions/3.10/lib/python3.10/typing.py", line 1081, in __getitem__
    return self.copy_with(tuple(new_args))
  File ".../Versions/3.10/lib/python3.10/typing.py", line 1294, in copy_with
    raise TypeError("The last parameter to Concatenate should be a "
TypeError: The last parameter to Concatenate should be a ParamSpec variable.

@erictraut
Copy link
Collaborator

Hmm, this looks like a bug in the runtime handling of Concatenate. You can work around it by enclosing the BulkFactory[...] in quotes or using from __future__ import annotations.

If I understand the problem you're trying to solve here, a ParamSpec isn't the right approach in the first place. A callback protocol would be much more appropriate.

class BulkFactory(Protocol):
    def __call__(
        self, count: int, /, id: int, *args: Any, **kwargs: Any
    ) -> List[Company]:
        ...

@erictraut
Copy link
Collaborator

The runtime exception appears to be a bug in the typing_extensions library. I've filed this issue to track this.

@lexicalunit
Copy link
Author

lexicalunit commented Jun 16, 2022

@erictraut I tried using the Protocol approach and that also has one issue that I can not resolve.

Example Code

from __future__ import annotations

from typing import Any, Callable, Generic, List, Optional, Protocol, TypeVar

from typing_extensions import ParamSpec, TypeAlias


def i_default() -> int:
    return 4


def s_default() -> str:
    return "foo"


class Company:
    id: int

    def __init__(
        self,
        id: Optional[int] = None,
        # ... dozens more optional properties go here
    ):
        self.id = id or i_default()  # each property has its own default
        print(self)

    def __repr__(self) -> str:
        return f"<company {self.id}>"


class User:
    id: int

    def __init__(
        self,
        uuid: Optional[str] = None,
        # ... dozens more optional properties go here
    ):
        self.uuid = uuid or s_default()  # each property has its own default
        print(self)

    def __repr__(self) -> str:
        return f"<user {self.uuid}>"


def make_company(*args: Any, **kwargs: Any) -> Company:
    return Company(*args, **kwargs)


def make_user(*args: Any, **kwargs: Any) -> User:
    return User(*args, **kwargs)


T = TypeVar("T")
P = ParamSpec("P")


class BulkFactory(Protocol, Generic[T]):
    def __call__(
        self,
        n: int,
        /,
        *args: Any,
        **kwargs: Any,
    ) -> List[T]:
        ...


def make_n(maker: Callable[P, T]) -> BulkFactory[T]:
    def inner(n: int, /, *args: P.args, **kwargs: P.kwargs) -> List[T]:
        return [maker(*args, **kwargs) for _ in range(n)]

    return inner


# create our bulk factories for user and company:
make_n_users = make_n(make_user)
make_n_companies = make_n(make_company)

# create a bunch of users and companies:
make_n_users(5)  # this should work, giving us all default values
make_n_users(5, uuid="whatever")  # this should work, giving us some
                                  # default values while others are set

make_n_companies(5)
make_n_companies(5, id=5)

Pylance Issue

On the line return inner we get:

(function) inner: (n: int, /, *args: P.args, **kwargs: P.kwargs) -> List[T@make_n]
Expression of type "(n: int, /, *args: P.args, **kwargs: P.kwargs) -> List[T@make_n]" cannot be assigned to return type "BulkFactory[T@make_n]"
  Type "(n: int, /, *args: P.args, **kwargs: P.kwargs) -> List[T@make_n]" cannot be assigned to type "(n: int, /, *args: Any, **kwargs: Any) -> List[T@make_n]"
    Parameter "**kwargs" has no corresponding parameter

EDIT:
I got this to work by adding P to the generic:

T = TypeVar("T")
P = ParamSpec("P")


class BulkFactory(Protocol, Generic[T, P]):
    def __call__(
        self,
        n: int,
        /,
        *args: P.args,
        **kwargs: P.kwargs,
    ) -> List[T]:
        ...


def make_n(maker: Callable[P, T]) -> BulkFactory[T, P]:
    def inner(n: int, /, *args: P.args, **kwargs: P.kwargs) -> List[T]:
        return [maker(*args, **kwargs) for _ in range(n)]

    return inner

Thank you!

@erictraut
Copy link
Collaborator

Yeah, that looks like a bug. Could you please open a new issue in the pyright issue tracker so this doesn't get lost? Thanks!

@lexicalunit
Copy link
Author

Oh it does? I figured I was just doing something wrong 😅 -- I'll open an issue now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
as designed Not a bug, working as intended
Projects
None yet
Development

No branches or pull requests

2 participants