ClassVar in protocols: mypy and pyright have different opinions

[tvannahl]

Hi,

I want to open a bug ticket in either mypy or pyright due to a behaviour which excludes usage of both together. I am not sure what the correct behaviour would be, that's why I am asking here.

Variant A

Considering the following code:

from typing import (
    Protocol,
    Type,
    List,
    runtime_checkable,
)


@runtime_checkable
class Plugin(Protocol):
    s: str

    def f(self):
        ...


class A:
    s = "Hello World"

    def f(self):
        print(A.s)


PLUGINS: List[Type[Plugin]] = [A, ]

if __name__ == '__main__':
    for p in PLUGINS:
        print(p.s)

mypy

Analytics with mypy do result in the following:

$ mypy mytest.py 
Success: no issues found in 1 source file
$ echo $?
0

pyright

Anatytics with pyright does result the following:

$ pyright mytest.py 
Loading configuration file at /app/pyrightconfig.json
Assuming Python version 3.10
Assuming Python platform Linux
Auto-excluding **/node_modules
Auto-excluding **/__pycache__
Auto-excluding .git
stubPath /app/typings is not a valid directory.
Searching for source files
Found 1 source file
0 errors, 0 warnings, 0 informations 
Completed in 0.649sec
bash-5.1# pyright mytest.py 
Loading configuration file at /app/pyrightconfig.json
Assuming Python version 3.10
Assuming Python platform Linux
Auto-excluding **/node_modules
Auto-excluding **/__pycache__
Auto-excluding .git
stubPath /app/typings is not a valid directory.
Searching for source files
Found 1 source file
/app/mytest.py
  /app/mytest.py:28:17 - error: Protocol class "Plugin" does not define "s" as a ClassVar (reportGeneralTypeIssues)
1 error, 0 warnings, 0 informations 
Completed in 0.681sec
$ echo $?
1

Variant B

Okay lets fix the pyright complaint regarding Variant A with the following code:

from typing import (
    ClassVar,
    Protocol,
    Type,
    List,
    runtime_checkable,
)


@runtime_checkable
class Plugin(Protocol):
    s: ClassVar[str]

    def f(self):
        ...


class A:
    s = "Hello World"

    def f(self):
        print(A.s)


PLUGINS: List[Type[Plugin]] = [A, ]

if __name__ == '__main__':
    for p in PLUGINS:
        print(p.s)

mypy

Now mypy complaints:

$ pipenv run mypy mytest.py
mytest.py:25: error: List item 0 has incompatible type "Type[A]"; expected "Type[Plugin]"
Found 1 error in 1 file (checked 1 source file)
$ echo $?
1

pyright

everythings fine here:

$ pyright mytest.py 
Loading configuration file at /app/pyrightconfig.json
Assuming Python version 3.10
Assuming Python platform Linux
Auto-excluding **/node_modules
Auto-excluding **/__pycache__
Auto-excluding .git
stubPath /app/typings is not a valid directory.
Searching for source files
Found 1 source file
0 errors, 0 warnings, 0 informations 
Completed in 0.669sec
$ echo $?
0

Question

What would you say, and why: Who did the better/correct conclusion and why?

[Akuli]

The following change makes it work in both:

@@ -16,7 +16,7 @@
 
 
 class A:
-    s = "Hello World"
+    s: ClassVar = "Hello World"
 
     def f(self):
         print(A.s)

For some reason, mypy seems to think that s = "Hello World" creates an instance variable.

Also, every time you wrote pylint, you actually meant pyright :)

[erictraut]

I presume that you mean pyright, not pylint in the title and the first sentence?

I'm the main author of pyright, so I'm biased :), but I think pyright is correct in this case.

As @Akuli said, mypy appears to think that s = "Hello World" is not a class variable when it clearly is. I wasn't able to find an existing bug report for this in the mypy issue tracker, so you might want to create one.

[tvannahl]

That has been my bad. I just started using pyright and it's friday. Great tool btw. - I fixed my text.

Thank you @Akuli I'll use your proposal to have both cases covered. Great to see that this is at least resolvable - for a moment I did fear that I would need to abandon one. I heard about such cases in other languages type hints.

[carljm]

Per PEP 526, mypy is right and pyright is wrong, so filing a bug on mypy would not make sense here. (See following thread for more details.)

[tvannahl]

Following my argument under the other thread, I am marking this as the solution: #1056 (reply in thread)

But I could not have came to it without so much great input.

[AlexWaygood]

As @Akuli says, the difference in behaviour here appears to be due to the fact that pyright infers s to be an implicit ClassVar, whereas mypy chooses only to understand a variable as a ClassVar if you explicitly tell it that a variable is a ClassVar.

As for which is correct: it's hard to say; it's a somewhat subjective question. It's likely that, in the majority of cases, pyright will be correct; s does look like it's probably a ClassVar, from my reading of your code. But there is code out there that assigns values in the class namespace, yet is nonetheless happy to have those names overridden on instances of that class. For example, the socketserver.BaseServer class in the stdlib assigns timeout = None in the class namespace (which pyright would infer as an implicit ClassVar). But the docstring of BaseServer explicitly states that it's fine for instances of BaseServer to override that value.

[carljm]

I suppose it can be expressed in a stub in the same way, by presence or absence of = ...? Eg x: int vs x: int = ... on the class.

[AlexWaygood]

I suppose it can be expressed in a stub in the same way, by presence or absence of = ...?

I suppose so. But it does seem strange that having a default value would have different semantics in a stub file depending on whether it was a module-level variable or a class-level variable. E.g. @JelleZijlstra just recently did a sweep removing nearly all = ... module-level defaults from typeshed as a matter of style, as they are meaningless to a type checker.

[erictraut]

Pyright internally differentiates between:

1. A class variable that can be overridden by an instance variable
2. A class variable that has been explicitly declared as a 'ClassVar' (and therefore cannot be overridden by an instance variable)
3. An instance variable that has no corresponding class variable

For purposes of protocol matching, it allows variables in category 1 to satisfy variables in category 2. I think this is generally safe. It's certainly practical.

Consider the following...

from typing import ClassVar

class ParentA:
    foo: int = 0

class ParentB:
    def __init__(self) -> None:
        self.foo: int = 0

class ParentC:
    foo: ClassVar[int] = 0

_ = ParentA.foo
_ = ParentA().foo

_ = ParentB.foo  # Pyright Error - accessing instance-only variable (runtime exception)
_ = ParentB().foo

_ = ParentC.foo
_ = ParentC().foo

ParentA.foo = 0
ParentA().foo = 0  # Allowed - instance variable overwrites class variable

ParentB.foo = 0  # Pyright Error - not a class variable
ParentB().foo = 0

ParentC.foo = 0
ParentC().foo = 0  # Pyright Error - instance variable cannot overwrite class variable

Pyright generates three errors above. Mypy generates none in normal mode and one (on the last line) in strict mode.

[tvannahl]

A lot of great points thank you all. I've chosen to use Protocol instead as it is similar to Rusts Trait system. "The big idea is messaging" and so - just to provide context for my motivation.

The following two sentences of PEP 526 seem to me the only parts than can be applied here:

In particular, the value-less notation a: int allows one to annotate instance
variables that should be initialized in __init__ or __new__.

and

Here ClassVar is a special class defined by the typing module that
indicates to the static type checker that this variable should not be set on
instances.

The first sentence together with the fact that I am working on classes rather than instances I do agree with @erictraut: Variant A should raise an error. I never made the promise that a Plugin class has the attribute s.

Personal Conclusion

After trying to find arguments for different versions I would say:

• Variant A is indeed wrong due to the reasons I did describe above. (pyright is correct)
• Variant B is indeed wrong, as I made different promises for protocol and implementation. (mypy is correct)

That's why I would say that @Akuli s solution is not good for making both tools shut up but is indeed the only consequent promise - although I would love to a a more expressive explanation from mypy.

Thank you all for your input and context. It did help me a great deal, hope you had some fun with it as I did.

---

I want to keep the following text in memoriam of all the text I wrote and deleted trying to find an conclusive argument:

***

[carljm]

Sorry, I somewhat misunderstood the original issue and pyright's behavior! I agree that both are defensible, but I do still find mypy's behavior slightly preferable here, because it is possible (if unlikely) that code relying on the protocol could rely on the fact that a ClassVar should not be overridable in an instance; all instances should share the same value. And it is relatively low-cost to require an explicit ClassVar to satisfy an explicit ClassVar in the protocol, as @Akuli's suggestion does.