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

Unable to match Protocol implementation using function-level generic to Protocol interface using class-level generic #5027

Closed
EthanBullVulpe opened this issue Apr 27, 2023 · 10 comments
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working

Comments

@EthanBullVulpe
Copy link

EthanBullVulpe commented Apr 27, 2023

Environment data

  • Language Server version: 2023.4.41
  • OS and version: linux x64
  • Python version (and distribution if applicable, e.g. Anaconda): 3.10 (Anaconda)
  • python.analysis.indexing: true
  • python.analysis.typeCheckingMode: strict

Code Snippet

from __future__ import annotations
from abc import abstractmethod
from typing import Protocol, Sequence, TypeVar

Input = TypeVar("Input", contravariant=True)
Output = TypeVar("Output", covariant=True)

T = TypeVar("T")

class Map(Protocol[Input, Output]): # the Protocol interface
    
    @abstractmethod
    def __call__(self, input: Input) -> Output:
        raise NotImplementedError
    
class GetFirst: # the Protocol implementation
    
    def __call__(self, input: Sequence[T]) -> T:
        return input[0]
    
def select(selector: Map[Sequence[int], int]) -> bool:
    raise NotImplementedError

select(GetFirst()) # This line produces a type error
    
m: Map[Sequence[int], int] = GetFirst() # This line produces a type error

Expected behavior

I think there should be no type error on lines select(GetFirst()) and m: Map[Sequence[int], int] = GetFirst(). Since GetFirst implements the Map[Sequence[T], T] Protocol based on the definition of GetFirst.__call__, and thus should match the type Map[Sequence[int], int].

Actual behavior

Pylance strict type checking gives an error on the following two lines:
select(GetFirst())
and
m: Map[Sequence[int], int] = GetFirst()
In both cases, the error is essentially the same:

Expression of type "GetFirst" cannot be assigned to declared type "Map[Sequence[int], int]"
  "GetFirst" is incompatible with protocol "Map[Sequence[int], int]"
    TypeVar "Input@Map" is contravariant
      "Sequence[int]" is incompatible with "Sequence[T@__call__]"
        TypeVar "_T_co@Sequence" is covariant
          Type "int" cannot be assigned to type "T@__call__"
    TypeVar "Output@Map" is covariant
      "object*" is incompatible with "int"

I have a feeling that the source of the issue is that in Map, Input and Output are class-level generics, while in GetFirst, T is a function-level generic, but I do not see why that should lead to a type error.

Note that mypy considers this code well-typed.

Conda Enviornment

[environment.yml.txt](https://github.com/microsoft/pylance-release/files/11348276/environment.yml.txt)

Thanks in advance.

@erictraut
Copy link
Collaborator

erictraut commented Apr 27, 2023

@rchiodo , would you like to transfer this to pyright since it's a core type checking issue?

@rchiodo
Copy link
Collaborator

rchiodo commented Apr 27, 2023

Will do :)

@rchiodo rchiodo transferred this issue from microsoft/pylance-release Apr 27, 2023
@rchiodo rchiodo removed their assignment Apr 27, 2023
@erictraut erictraut added the bug Something isn't working label Apr 28, 2023
@erictraut erictraut changed the title Pylance type checker unable to match Protocol implementation using function-level generic to Protocol interface using class-level generic. Unable to match Protocol implementation using function-level generic to Protocol interface using class-level generic Apr 28, 2023
@last-partizan
Copy link
Contributor

Looks like i stumbled upon this bug as well, or at least similar one.

Here's real life example, djangoresframework uses djangoresftramework-stubs, and it has defined protocol for filter backend classes:

_Q = TypeVar("_Q", bound=Union[QuerySet[Any], MongoQuerySet[Any]])

class _FilterBackendProtocol(Protocol):
    def filter_queryset(self, request: Any, queryset: _Q, view: APIView) -> _Q: ...

But when using with untyped package django-filters, it produces error: "DjangoFilterBackend" is incompatible with protocol "_FilterBackendProtocol"

# This is from reveal_type, for the reference
Type of "DjangoFilterBackend.filter_queryset" is "(self: DjangoFilterBackend, request: Unknown, queryset: Unknown, view: Unknown) -> (Unknown | Any | QuerySet[Unknown])"

Full example here:

https://github.com/last-partizan/pyright-examples/tree/44e045de106e3d5f2fe7f079280441eca1654be2

@erictraut is that looks like the same bug, or i'd better create new issue?

@erictraut
Copy link
Collaborator

@last-partizan, yes, I can confirm this is the same issue.

@Hootner

This comment was marked as spam.

@erictraut
Copy link
Collaborator

@last-partizan, upon further investigation, this isn't the same issue. The problem is that the DjangoFilterBackend method filter_queryset is unannotated, so its return type is being inferred. This return type isn't compatible with the protocol _FilterBackendProtocol. So I think pyright is correct to generate an error in this case. To fix this, you'd need to provide correct type information for the django_filters library in the form of type stubs.

erictraut pushed a commit that referenced this issue Jun 18, 2023
@erictraut
Copy link
Collaborator

The bug reported at the top of this issue will be addressed in the next release.

@erictraut erictraut added the addressed in next version Issue is fixed and will appear in next published version label Jun 18, 2023
@last-partizan
Copy link
Contributor

I'm trying to figure out proper type annotation, but something is not right.

Pyright version tested:

  • 1.1.308
  • 1.1.314
  • built from source (main)

Here's my example, stripped out of all external references.

from typing import Any, Generic, Protocol, TypeVar

T = TypeVar("T")


class QuerySet(Generic[T]):
    ...


Q = TypeVar("Q", bound=QuerySet[Any])


class FilterBackendProtocol(Protocol):
    def filter_queryset(self, qs: Q) -> Q:
        ...


class DjangoFilterBackend:
    def filter_queryset(self, qs: QuerySet[Any]) -> QuerySet[Any]:
        return qs


class APIView:
    filter_backends: list[type[FilterBackendProtocol]]


class FinalAPIView(APIView):
    filter_backends = [DjangoFilterBackend]
/home/serg/src/pyright-examples/example.py
  /home/serg/src/pyright-examples/example.py:28:24 - error: Expression of type "list[type[DjangoFilterBackend]]" cannot be assigned to declared type "list[type[FilterBackendProtocol]]"
    "DjangoFilterBackend" is incompatible with protocol "FilterBackendProtocol"
    Type "type[DjangoFilterBackend]" cannot be assigned to type "type[FilterBackendProtocol]"
      "filter_queryset" is an incompatible type
        Type "(qs: QuerySet[Any]) -> QuerySet[Any]" cannot be assigned to type "(qs: Q@filter_queryset) -> Q@filter_queryset"
          Function return type "QuerySet[Any]" is incompatible with type "Q@filter_queryset"
            Type "QuerySet[Any]" cannot be assigned to type "Q@filter_queryset" (reportGeneralTypeIssues)
1 error, 0 warnings, 0 informations

@erictraut
Copy link
Collaborator

@last-partizan, the problem with your most recent code sample is that your implementation of DjangoFilterBackend.filter_queryset doesn't meet the requirements of the protocol. The filter_queryset method in the protocol requires that the type of the qs parameter and the return type are identical because they share the same type variable. In your implementation, the qs parameter can be any subtype of QuerySet[Any], and the return type can be a different subtype of QuerySet[Any]. To make your implementation compatible with the protocol, you'd need to use a type variable in its signature.

class DjangoFilterBackend:
    def filter_queryset(self, qs: Q) -> Q:
        return qs

last-partizan added a commit to last-partizan/djangorestframework-types that referenced this issue Jun 20, 2023
After latest changes to pyright, old code isn't working with django-filter.
microsoft/pyright#5027 (comment)

Instead of adding types to django-filter, we modify this to allow any QS
last-partizan added a commit to last-partizan/djangorestframework-types that referenced this issue Jun 20, 2023
After latest changes to pyright, old code isn't working with django-filter.
microsoft/pyright#5027 (comment)

Instead of adding types to django-filter, we modify this to allow any QS
jbradaric added a commit to jbradaric/pyright-inlay-hints that referenced this issue Jun 20, 2023
commit 63f5658
Author: Erik De Bonte <[email protected]>
Date:   Tue Jun 20 10:50:18 2023 -0700

    Support PEP 712's new attribute assignment conversion (microsoft#5343)

commit 9cbe8c0
Author: Eric Traut <[email protected]>
Date:   Tue Jun 20 09:17:40 2023 -0700

    Fixed a bug that caused an incorrect false positive error for an `assert_type` call involving a `Callable[[], X]` type. Pyright was generating a signature with a positional-only separator in this case. This addresses python/typeshed#10325 (comment). (microsoft#5345)

    Co-authored-by: Eric Traut <[email protected]>

commit d12ad6e
Author: Eric Traut <[email protected]>
Date:   Mon Jun 19 23:59:08 2023 -0700

    Updated typeshed stubs to the latest version.

commit eff69f5
Author: Eric Traut <[email protected]>
Date:   Mon Jun 19 22:35:56 2023 -0700

    Fixed a bug that caused incorrect type inference for parameters in unannotated methods within child classes who derive from a generic parent class. This addresses a bug reported on stack overflow: https://stackoverflow.com/questions/76466874/trouble-specifying-type-parameter-when-inheriting-from-generic-class.

commit 43c5fae
Author: Eric Traut <[email protected]>
Date:   Mon Jun 19 20:17:00 2023 -0700

    Finished cleanup of test cases.

commit b0d4080
Author: Eric Traut <[email protected]>
Date:   Mon Jun 19 19:15:53 2023 -0700

    Next batch of test case cleanup.

commit efea9b9
Author: Eric Traut <[email protected]>
Date:   Mon Jun 19 18:26:49 2023 -0700

    Changed behavior of non-ClassVar variables within a protocol definition. Previously, an error was reported when such variables were accessed from the class (as opposed to an instance of the class). Mypy (which was the reference implementation for PEP 544) does not report an error here. This addresses microsoft/pylance-release#4389.

commit 5568b4d
Author: Eric Traut <[email protected]>
Date:   Mon Jun 19 18:00:31 2023 -0700

    Enhanced command-line version of pyright to allow file or directory names to be passed via stdin if `-` option is used in the command line. This addresses microsoft#5342.

commit cd257f7
Author: Eric Traut <[email protected]>
Date:   Mon Jun 19 15:38:13 2023 -0700

    Next batch of test case cleanup.

commit 0c5c7b6
Author: Eric Traut <[email protected]>
Date:   Mon Jun 19 14:23:50 2023 -0700

    Next batch of text case cleanup.

commit 899799d
Author: Eric Traut <[email protected]>
Date:   Mon Jun 19 13:33:41 2023 -0700

    Finished cleaning up "genericTypesX.py" test files.

commit 1111f1b
Author: Eric Traut <[email protected]>
Date:   Mon Jun 19 11:49:56 2023 -0700

    Started to untangle the hairball related to the "genericTypesX.py" test cases.

commit 67394ad
Author: Eric Traut <[email protected]>
Date:   Mon Jun 19 10:21:36 2023 -0700

    Continued cleanup of test cases.

commit 96e03aa
Author: Eric Traut <[email protected]>
Date:   Mon Jun 19 09:39:09 2023 -0700

    Did a cleanup pass for the dataclass test cases. Renamed and reordered test cases for maintainability.

commit b7df200
Author: Eric Traut <[email protected]>
Date:   Mon Jun 19 08:20:54 2023 -0700

    Fixed bug that resulted in a false positive error when defining a new type alias using the `TypeAliasType` constructor that defines no new type parameters but references an outer-scoped type parameter in the type alias definition. This addresses microsoft#5341.

commit d816d00
Author: Eric Traut <[email protected]>
Date:   Mon Jun 19 00:00:07 2023 -0700

    Started to do a pass over the test cases to make them more consistent.

commit 23bcbce
Author: Eric Traut <[email protected]>
Date:   Sun Jun 18 17:42:25 2023 -0700

    Fixed bug in code flow engine that led to incorrect type evaluation of a variable in a nested loop. This addresses https://github.com/microsoft/pylance-release/issues/4509.

commit 056415f
Author: Eric Traut <[email protected]>
Date:   Sun Jun 18 16:40:00 2023 -0700

    Fixed a bug in the control flow debugging code that prints the control flow graph. It was not correctly handling one of the node types which led to incomplete graphs.

commit 643bb1d
Author: Eric Traut <[email protected]>
Date:   Sun Jun 18 13:20:49 2023 -0700

    Improved type inference for lambdas in the case where a parameter includes a default value and the expected type doesn't include that parameter. This improvement was suggested in the [mypy issue tracker](python/mypy#15459). (microsoft#5337)

    Co-authored-by: Eric Traut <[email protected]>

commit 8ce23eb
Author: Eric Traut <[email protected]>
Date:   Sun Jun 18 12:55:22 2023 -0700

    Improved `reportUnnecessaryCast` so it works with types other than cl… (microsoft#5336)

    Improved `reportUnnecessaryCast` so it works with types other than class instances. This addresses microsoft#5333.

commit 52c8cac
Author: Eric Traut <[email protected]>
Date:   Sun Jun 18 12:41:24 2023 -0700

    Changed type printer (the component that renders types into text) to use the lowercase `type[x]` instead of `Type[x]`. It has now been four years since PEP 585 deprecated the use of the upper-case version, so most developers should be getting comfortable with the lowercase version at this point.

commit e3080b1
Author: Eric Traut <[email protected]>
Date:   Sun Jun 18 10:15:58 2023 -0700

    Fixed a bug that led to a false positive error under certain circumstances when a literal type argument was used in conjunction with a protocol that used a covariant type parameter and an implementation of that protocol that used an invariant type parameter. This addresses microsoft#5282. (microsoft#5332)

    Co-authored-by: Eric Traut <[email protected]>

commit 2b0f8d2
Author: Eric Traut <[email protected]>
Date:   Sun Jun 18 09:46:18 2023 -0700

    Improved hover text to display the calculated variance for a PEP 695-style class-scoped type variable when the user hovers over the type parameter in the type param list.

commit 02b7769
Author: Eric Traut <[email protected]>
Date:   Sun Jun 18 00:17:53 2023 -0700

    Fixed a false positive error arising from the use of a binary expression for a base class in a class declaration statement. This addresses microsoft#5326. (microsoft#5331)

    Co-authored-by: Eric Traut <[email protected]>

commit 2de35e3
Author: Eric Traut <[email protected]>
Date:   Sun Jun 18 00:00:06 2023 -0700

    Added documentation about higher-order functions.

commit 42a37f4
Author: Eric Traut <[email protected]>
Date:   Sat Jun 17 23:38:50 2023 -0700

    Re-enabled a test case that was previously disabled because it was broken.

commit 7ea11a1
Author: Eric Traut <[email protected]>
Date:   Sat Jun 17 23:17:32 2023 -0700

    Minor code cleanup — rename constant for clarity and refactor validation function. No functional change.

commit 53cb3f9
Author: Eric Traut <[email protected]>
Date:   Sat Jun 17 23:00:49 2023 -0700

    Improved consistency of parameter ordering internally to type evaluator. No functional change.

commit 9bf8231
Author: Eric Traut <[email protected]>
Date:   Sat Jun 17 22:48:18 2023 -0700

    Fixed a bug that led to incorrect type evaluation when passing a generic class (with a constructor that includes class-scoped TypeVars) as an argument for a callable parameter. The class was being specialized prematurely (with type arguments set to `Unknown`) before the constraint solver was able to solve the higher-order function's type variables. This addresses microsoft#5324. (microsoft#5328)

    Co-authored-by: Eric Traut <[email protected]>

commit 47cd514
Author: Eric Traut <[email protected]>
Date:   Sat Jun 17 22:21:52 2023 -0700

    Changed auto-variance algorithm to ignore `__new__` and `__init__` methods for purposes of calculating the variance of a TypeVar. This mirrors the behavior of mypy. (microsoft#5327)

    Co-authored-by: Eric Traut <[email protected]>

commit 3021b9c
Author: Eric Traut <[email protected]>
Date:   Sat Jun 17 21:52:57 2023 -0700

    Minor code cleanup — removed dead code and converted lambda to function. No functional change.

commit 327ce37
Author: Eric Traut <[email protected]>
Date:   Sat Jun 17 20:30:36 2023 -0700

    Added test case for microsoft#5027.

commit 0559382
Author: Eric Traut <[email protected]>
Date:   Sat Jun 17 14:19:12 2023 -0700

    Fixed false negative when a literal and non-literal are assigned to the same TypeVar in an invariant context. This addresses microsoft#5321. (microsoft#5323)

    Co-authored-by: Eric Traut <[email protected]>

commit 2829429
Author: Eric Traut <[email protected]>
Date:   Sat Jun 17 12:33:13 2023 -0700

    Fixed bug that led to an incorrect type evaluation for nested call expressions where an inner call expression used a ParamSpec. This addresses microsoft#5281. (microsoft#5322)

    Co-authored-by: Eric Traut <[email protected]>
@erictraut
Copy link
Collaborator

This is included in pyright 1.1.315, which I just published. It will also be included in this week's insiders release of pylance.

sbdchd pushed a commit to sbdchd/djangorestframework-types that referenced this issue Jul 19, 2023
…ht (#38)

After latest changes to pyright, old code isn't working with django-filter. microsoft/pyright#5027 (comment)

Instead of adding types to django-filter, we modify this to allow any QS

We also removing schema-related methods from protocols, becouse they're already deprecated in django-filters.

Other options that i have considered - adding type-var bound types to django-filter package, but that would be harder to implement, so i think relaxing requirements here is better.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants