Inherited factory function annotation

[bloussou]

Hello,
The following sample of code doesn't throw any error in 1.101 but does throw in 1.102 because this Bug Fix: Fixed reportIncompatibleMethodOverride diagnostic check so it doesn't ignore incompatible protected methods (those whose names start with a single underscore). was added in the release.

The error is :

Method "_from_dict" overrides class "FromDict" in an incompatible manner
    Parameter 1 type mismatch: base parameter is type "Type[_FromDict@_from_dict]", override parameter is type "Type[_Configuration@_from_dict]"

The code to reproduce :

from __future__ import annotations

from abc import ABC, abstractmethod
from typing import Any, Mapping, Type, TypeVar

_FromDict = TypeVar("_FromDict", bound="FromDict")

class FromDict(ABC):

    @classmethod
    @abstractmethod
    def _from_dict(
        cls: Type[_FromDict], data: Mapping[str, Any]
    ) -> _FromDict:
        pass

_Configuration = TypeVar("_Configuration", bound="Configuration")

class Configuration(FromDict):

    @classmethod
    @abstractmethod
    def _create(cls: Type[_Configuration], data: Mapping[str, Any]) -> _Configuration:
        """"""

    @classmethod
    def _from_dict(cls:Type[_Configuration], data: Mapping[str, Any]) -> _Configuration:
        return cls._create({}) #type: ignore

I want to have the method _from_dict to return the type of classes extending Configuration (a sort of factory function). What am I doing wrong ?

Thanks for your help.

[erictraut]

Your code looks correct to me. Looks like a bug in pyright. I'll look into it. Feel free to file an issue if you'd like to receive updates on the investigation and fix. Thanks!

[erictraut]

Incidentally, I don't think this has anything to do with the reportIncompatibleMethodOverride diagnostic check. It repros with the following (simpler) example:

from typing import Any, Mapping, Type, TypeVar

_Configuration = TypeVar("_Configuration", bound="Configuration")


class Configuration:
    @classmethod
    def _create(cls: Type[_Configuration], data: Mapping[str, Any]) -> _Configuration:
        ...

    @classmethod
    def _from_dict(
        cls: Type[_Configuration], data: Mapping[str, Any]
    ) -> _Configuration:
        return cls._create({})

[bloussou]

Thanks a lot. I have open #1500.

[bloussou]

Hello,
I think this is not fully fixed. In 1.113 it's possible to remove the #type : ignore but there is still issue in Configuration._from_dict.
To reproduce :

from __future__ import annotations

from typing import Any, Mapping, Type, TypeVar

from abc import ABC, abstractmethod
from typing import Any, Mapping, Type, TypeVar

_FromDict = TypeVar("_FromDict", bound="FromDict")

class FromDict(ABC):

    @classmethod
    @abstractmethod
    def _from_dict(
        cls: Type[_FromDict], data: Mapping[str, Any]
    ) -> _FromDict:
        pass

_Configuration = TypeVar("_Configuration", bound="Configuration")
class Configuration(FromDict):
    @classmethod
    def _create(cls: Type[_Configuration], data: Mapping[str, Any]) -> _Configuration:
        ...

    @classmethod
    def _from_dict(
        cls: Type[_Configuration], data: Mapping[str, Any]
    )-> _Configuration:
        return cls._create({})

The error :
Method "_from_dict" overrides class "FromDict" in an incompatible manner Parameter 1 type mismatch: base parameter is type "Type[_FromDict@_from_dict]", override parameter is type "Type[_Configuration@_from_dict]"

[erictraut]

This override arguably violates the Liskov substitution principle. Consider this simpler example:

# pyright: strict

class Parent:
    def method1(self, data: float): ...
    def method2(self, data: int): ...

class Child(Parent):
    # This violates the Liskov substitution principle because
    # int is a subtype of float.
    def method1(self, data: int): ...

    # This does not violate the Liskov substitution principle because
    # int is a subtype of int.
    def method2(self, data: float): ...

That said, I think it's safe in the case of self or cls to violate this principle. Mypy appears to allow it.

I've updated the logic to allow this special case.

[bloussou]

Thanks for your help !