Abstract class properties report inconsistent typing

[Dr-Irv]

Describe the bug
This is related to microsoft/pylance-release#435 but with the latest version of pyright, types seem inconsistent and I'm getting new messages that weren't there before.

To Reproduce

from abc import ABC, abstractmethod
from typing import List, Type, Dict


class BaseModel(ABC):
    @classmethod
    @property
    @abstractmethod
    def model_code(cls) -> str:
        pass

    # model_code: str


class M1(BaseModel):
    model_code = "m1"


class M2(BaseModel):
    model_code = "m2"


reveal_type(BaseModel.model_code)
reveal_type(M1.model_code)


class ModelMapper:
    def __init__(self, model_list: List[Type[BaseModel]]):
        self.__registry: Dict[str, Type[BaseModel]] = {
            mc.model_code: mc for mc in model_list
        }

    def get_model_class(self, model_code: str) -> Type[BaseModel]:
        return self.__registry[model_code]

    def add_model_class(self, model_class: Type[BaseModel]):
        self.__registry[model_class.model_code] = model_class


mm = ModelMapper([M1, M2])
for m in ["m1", "m2"]:
    print("code: ", m, " class: ", mm.get_model_class(m))

This reports:

Expression of type "Literal['m1']" cannot be assigned to declared type "property"
  "Literal['m1']" is incompatible with "property"
Expression of type "Literal['m2']" cannot be assigned to declared type "property"
  "Literal['m2']" is incompatible with "property"
Type of "BaseModel.model_code" is "str"
Type of "M1.model_code" is "str"

Note that the error message says you can't assign them (the base class has type property, but it should be "abstract class property that is a str", but reveal_type says they are both str, which intuitively is saying you should allow that assignment.

Expected behavior
I would like it where the assignment was allowed. Idea here is that the model_code becomes a lookup telling you which class to instantiate.

VS Code extension or command-line
VS Code 1.62.3
pylance 2021.11.2

Additional context
I know there has been debate in multiple issues (e.g., microsoft/pylance-release#157 and microsoft/pylance-release#435) about how to handle abstract class properties, with arguments that say you are overriding the type of model_code in the subclass, where the parent class sees it as a property, and the subclass sees it as str, but I don't see how to make this work with typing where I accomplish the following goals:

• Tell the developer they have to define the property value in the concrete class
• Allow a dynamic registry of classes based on "codes" that indicate each class

Note that if I change BaseModel to just have model_code: str rather than the abstract class property, the code still works, but then I don't accomplish the first goal listed above.

Maybe the reveal_type(BaseModel.model_code) is what is being reported incorrectly, which is a bug to fix, so then you could see the mismatch in types. mypy reports it as Revealed type is "def () -> builtins.str"

But I would still like to figure out how to accomplish my goals....

[erictraut]

The abstract base class indicates that subclasses need to implement a classmethod property, but in this sample they're overwriting the model_code symbol with an attribute. Properties and attributes have similar semantics, but they're not the same. Especially in this case where the property is read-only and the attribute is writable. So I think pyright is doing the right thing here.

I can think of two options here:

1. Subclasses can implement the property defined in the base class.

class M1(BaseModel):
    @classmethod
    @property
    def model_code(cls) -> str:
        return "m1"

2. You can switch from an abstract base class to a protocol.

class BaseModel(Protocol):
    model_code: ClassVar[str]

class M1:
    model_code = "m1"

class M2:
    model_code = "m2"

[Dr-Irv]

The abstract base class indicates that subclasses need to implement a classmethod property, but in this sample they're overwriting the model_code symbol with an attribute. Properties and attributes have similar semantics, but they're not the same. Especially in this case where the property is read-only and the attribute is writable. So I think pyright is doing the right thing here.

Except then why does reveal_type(BaseModel.model_code) show the type as str instead of it being a property?

I can think of two options here:

1. Subclasses can implement the abstract property defined in the base class.

class M1(BaseModel):
    @classmethod
    @property
    def model_code(cls) -> str:
        return "m1"

Yes, I know that will work, but it just doesn't look as clean as the original solution I had...

2. You can switch from an abstract base class to a protocol.

class BaseModel(Protocol):
    model_code: ClassVar[str]

class M1:
    model_code = "m1"

class M2:
    model_code = "m2"

That doesn't accomplish my first goal. Consider:

from typing import Protocol


class BaseModel(Protocol):
    model_code: str


class M1(BaseModel):
    pass
    # model_code = "m1"


class M2(BaseModel):
    model_code = "m2"


m1 = M1()

pylance (with basic type checking on) doesn't report any error, while mypy does say Cannot instantiate abstract class "M1" with abstract attribute "model_code"

Ideally, the type checker would pick up that the subclass didn't define everything it should as represented by the parent class that is a Protocol .

[Dr-Irv]

@erictraut sorry I thought I had posted this comment and then you closed the issue...

[erictraut]

reveal_type(BaseModel.model_code) reveals str because that's how class properties work. They are evaluated when accessed from the class.

In the protocol case, you wouldn't subclass from the protocol. In other words, M1 would not subclass from BaseClass in your example. That way, if the class in question doesn't implement the required field (model_code), you will receive errors.

from typing import Protocol

class BaseModel(Protocol):
    model_code: str

class M1:
    pass

class M2:
    model_code = "m2"

m1: BaseModel = M1() # Error!

[Dr-Irv]

reveal_type(BaseModel.model_code) reveals str because that's how class properties work. They are evaluated when accessed from the class.

So the confusion arises because the error message says:

Expression of type "Literal['m1']" cannot be assigned to declared type "property"
  "Literal['m1']" is incompatible with "property"

but reveal_type(BaseModel.model_code) reveals str.

In other words, the error message is saying the declared type is "property", but reveal_type() is saying str. So maybe there is a way to improve the error messages.

In the protocol case, you wouldn't subclass from the protocol. In other words, M1 would not subclass from BaseClass in your example. That way, if the class in question doesn't implement the required field (model_code), you will receive errors.

Thanks. Now I get it. The example I saw somewhere on the web misled me in terms of how Protocol works.

[xavdid]

I'm having a little trouble wrapping my head around this. Is there a way to do this that both gives runtime protections and makes pylance happy? I've got this:

class WithProp:
    @property
    def name(self) -> str:
        raise NotImplementedError()


class NoProp(WithProp):
    name = "cool"

which works great at runtime - if someone doesn't overwrite name, they get a loud error. But, I'm getting the Expression of type "Literal['cool']" cannot be assigned to declared type "property" error discussed here and in microsoft/pylance-release#224. Is there a way to write this that gives both a runtime error and doesn't light up static typing tooling?

Thanks much!

[erictraut]

Since the base class implements a read-only property (a getter with no setter), the child class should do the same.

class NoProp(WithProp):
    @property
    def name(self):
        return "cool"

[xavdid]

Got it! I appreciate the response.

I think it makes sense as the "most correct" approach typing wise, but this feels like a pretty common Python pattern. It would be great to be able to write the above code without typing complaints.

[dgrunwald]

Workaround: If the base class is defined as:

class WithProp:
    @property
    def name(self) -> str:  # type: ignore
        raise NotImplementedError()
    
    name: typing.ClassVar[str]

then derived classes can use either properties or simple members without triggering additional type errors.

[mikenerone]

Note regarding that workaround: the type declaration must come after the property definition as shown in the example. Thanks for the workaround, @dgrunwald!