Automatically infer class variables based on declaration

[FeldrinH]

There seems to be a limitation with Pylance detecting if a variable declared in a class is a class variable or not.

In particular, the following code:

from typing import ClassVar, Protocol

class SimpleProtocol(Protocol):
    a: ClassVar[str]

class SimpleClass:
    a = "test"

val: SimpleProtocol = SimpleClass()

Gives the following error:

As far as I understand, SimpleClass.a is in fact a class variable (and using it as such does not produce any runtime errors),

Changing SimpleClass definition to

class SimpleClass:
    a: ClassVar = "test"

gets rid of the error.

It would be nice if Pylance could automatically infer that a is a class variable without the need for explicit type annotations.

Tested with Pylance language server 2023.5.30 (pyright e1b6074b).

[erictraut]

Pyright, the static code analyzer upon which pylance is built, recognizes three types of class-scoped variables: 1) Pure class variables, 2) Regular class variables, and 3) Pure instance variables.

In your code sample, your protocol class has indicated that a needs to be a pure class variable. In your SimpleClass class definition, you've defined it as a regular class variable. That's why it's deemed incompatible during protocol matching.

As you noticed, you can eliminate the mismatch by specifying that SimpleClass.a is a pure class variable by adding an explicit ClassVar to the annotation. You can also eliminate the mismatch by changing SimpleProtocol.a to be a regular class variable (i.e. eliminate the ClassVar from the annotation).

[FeldrinH]

I'm a little confused.

If I understand you correctly, then this declares SimpleProtocol.a to be a regular class variable:

class SimpleProtocol(Protocol):
    a: str

Yet, with this declaration the following gives an error:

from typing import Protocol, Type

class SimpleProtocol(Protocol):
    a: str

def black_box() -> Type[SimpleProtocol]:
    raise NotImplementedError

typ: Type[SimpleProtocol] = black_box()
print(typ.a)

And the following does not give an error:

from typing import Protocol

class SimpleProtocol(Protocol):
    a: str

class SimpleClass:
    def __init__(self):
        self.a = "test"

val: SimpleProtocol = SimpleClass()

So in what sense is SimpleProtocol.a a 'regular class variable'? In both these examples the type checking seems to behave as if a was a pure instance variable.

[erictraut]

PEP 544 (which introduced Protocol classes) doesn't provide a way to distinguish between the three different types of class-level variables. It introduced ClassVar, which is clearly meant for "pure class variables". For other variables, it seems to indicate that they should be treated as pure instance variables.

To distinguish between protocol class variables and protocol instance variables, the special ClassVar annotation should be used as specified by PEP 526

However, now that I re-read that statement, I realize that it leaves some room for interpretation.

I just checked with mypy, another type checker that was the reference implementation for PEP 544, and it appears to use the following interpretation for protocols:

class SimpleProtocol(Protocol):
    a: ClassVar[str] # Pure class variable
    b: str # Regular class variable

    def __init__(self):
        self.c: str # Pure instance variable

Currently, pyright treats b as a pure instance variable within a protocol (consistent with my reading of the PEP), and that's what's leading to the error you're seeing.

It would be more consistent if protocol and non-protocol classes worked in a consistent manner, and if that doesn't violate PEP 544 (as it apparently does not), then I'm happy to make that change in pyright. That would also make pyright consistent with mypy in this regard.

Could someone on the pylance team please transfer this issue to the pyright repo?

[FeldrinH]

FWIW the example in my original post does typecheck in Pycharm with no errors and no warnings.

EDIT: To avoid any possible confusion. This is the example that passes all Pycharm typechecks:

from typing import ClassVar, Protocol

class SimpleProtocol(Protocol):
    a: ClassVar[str]

class SimpleClass:
    a = "test"

val: SimpleProtocol = SimpleClass()

[FeldrinH]

Also probably worth noting:
The mypy interpretation of class and instance variables seems to introduce a different issue.

The following code passes mypy --strict with no issues found, but raises an error at runtime when accessing typ.a:

from typing import Protocol, Type

class SimpleProtocol(Protocol):
    a: str

class SimpleClass:
    def __init__(self) -> None:
        self.a = "test"

def black_box() -> Type[SimpleProtocol]:
    return SimpleClass

typ: Type[SimpleProtocol] = black_box()
print(typ.a)

[erictraut]

Yes, that's a general problem with "regular class variables" in Python. There's generally no way for a static type checker to know whether they have been initialized at the class or instance level (or both). Most other programming languages (for good reason) force you to declare whether a variable is a class or instance variable, and they don't permit it to be both.

[FeldrinH]

Fair enough. The main question that I am still unclear on is this:

If Pyright was changed to consider the following code as valid and well typed, would it violate some PEP or lead to problematic soundness issues?

from typing import ClassVar, Protocol

class SimpleProtocol(Protocol):
    a: ClassVar[str]

class SimpleClass:
    a = "test"

val: SimpleProtocol = SimpleClass()

[erictraut]

If Pyright was changed to consider the following code as valid and well typed, would it violate some PEP or lead to problematic soundness issues?

I think this once again depends on how PEP 544 is interpreted. Looking to mypy (which, as I mentioned, was the reference implementation for PEP 544), it generates an error in this case. I'm struggling to reconcile mypy's behaviors. It seems to treat a = "test" as a "pure instance variable" in this case (which doesn't make sense because it's clearly being assigned as a class variable). It interprets it as a "regular class variable" in other cases.

Setting aside mypy's inconsistent behavior and thinking about this from first principles, I agree with your assertion that this should be sound from a typing perspective. SimpleClass.a should be interpreted as a "regular class variable", which satisfies the "pure class variable" defined in the protocol. (Note: the converse situation would not be type safe.)

My conclusion is that there are two changes that I should make in pyright:

1. Remove the special-case handling for variables declared within the class body of a protocol class so they are categorized as "regular class variables" (as opposed to "pure instance variables") if they do not include a ClassVar annotation.
2. When performing protocol type consistency checks, allow a "regular class variable" to satisfy a protocol that defines the symbol as a "pure class variable".

Does that sound right to you?

[FeldrinH]

That does sound right to me. For the first change I don't really have a strong opinion either way. I fully agree with the second change and would be very happy if it was implemented.

[erictraut]

I'm going to hold off on the first change and give it more thought.

I've implemented the second change, and this will be in next week's release of pyright (and the insider build of pylance).

[gramster]

Did you decide about the first change Eric? Wondering if we can close this now.

[erictraut]

Apologies, I forgot about this issue. After thinking about this more, I think we should go ahead and make the first change as well. This will be included in the next release of pyright — and the next insiders build of pylance. You can close this issue after this week's pylance is released.

[gramster]

Thanks!

…

On Mon, Jun 19, 2023 at 6:28 PM Eric Traut ***@***.***> wrote: Apologies, I forgot about this issue. After thinking about this more, I think we should go ahead and make the first change as well. This will be included in the next release of pyright — and the next insiders build of pylance. You can close this issue after this week's pylance is released. — Reply to this email directly, view it on GitHub <#4389 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAVCPCDYUL3QJD73WIS3FQDXMD4ELANCNFSM6AAAAAAYG4ADL4> . You are receiving this because you commented.Message ID: ***@***.***>

[heejaechang]

This issue has been fixed in prerelease version 2023.6.31, which we've just released. You can find the changelog here: CHANGELOG.md