python / mypy

Optional static typing for Python
https://www.mypy-lang.org/
Other
18.57k stars 2.84k forks source link

PEP 702 (@deprecated): overriding deprecated methods #18085

Open tyralla opened 4 weeks ago

tyralla commented 4 weeks ago

I think some points need discussion before implementing everything neatly, especially regarding overloads. So, my first (untidy) commit focuses on "normal" methods (testDeprecatedOverriddenMethod).


Is it okay to override a deprecated method with another signature when not using @override?

My tendency is yes; see testDeprecatedOverriddenMethod.


Is it desirable that Mypy emits an error for C.f in the following case (as it currently does)?

class A:
    def f(self) -> int: ...
class B(A):
    def f(self) -> str: ..  # error !!!
class C(B):
    def f(self) -> str: ..  # error ???

My tendency is that no error should be emitted for C.f. I did not change this, but it appears to be simple.

I am asking because:

class A:
    @deprecated
    def f(self) -> int: ...
class B(A):
    def f(self) -> str: ..  # maybe okay, see above
class C(B):
    @override
    def f(self) -> str: ..  # should then be also okay

PEP 698 does not mention @overload. In Mypy, a single @override for any overload item or the implementation affects the whole overloaded method. This was decided here without any discussions I know of and will likely result in some inconsistencies with the overload item-specific implementation of @deprecated:

class A:
    @overload
    @deprecated("replaced by g")
    def f(self, x: int) -> None: ...
    @overload
    def f(self, x: str) -> None: ...
    @overload
    def f(self, x: None) -> None: ...
    def f(self, x: Union[int, str, None]) -> None: ...

class B:
    @overload
    @override
    def f(self, x: str) -> None: ...
    @overload
    @override
    def f(self, x: None) -> None: ...
    def f(self, x: Union[str, None]) -> None: ...

I think it would be favourable to neither emit deprecation nor signature-incompatible notes in the given example. (I did not investigate how hard it would be to implement this. override and deprecated are properties of symbols. But maybe we could just remove some items before doing the checks.)

tyralla commented 4 weeks ago

@JelleZijlstra: I am not allowed to add the correct label, am I?

github-actions[bot] commented 4 weeks ago

Diff from mypy_primer, showing the effect of this PR on open source code:

Tanjun (https://github.com/FasterSpeeding/Tanjun)
+ tanjun/context/slash.py:1045: note:      Superclass:
+ tanjun/context/slash.py:1045: note:          Optional[ExecutableCommand[SlashContext]]
+ tanjun/context/slash.py:1045: note:      Subclass:
+ tanjun/context/slash.py:1045: note:          Optional[BaseSlashCommand]
+ tanjun/context/message.py:130: note:      Superclass:
+ tanjun/context/message.py:130: note:          Optional[ExecutableCommand[MessageContext]]
+ tanjun/context/message.py:130: note:      Subclass:
+ tanjun/context/message.py:130: note:          Optional[MessageCommand[Any]]
+ tanjun/context/menu.py:101: note:      Superclass:
+ tanjun/context/menu.py:101: note:          Optional[ExecutableCommand[MenuContext]]
+ tanjun/context/menu.py:101: note:      Subclass:
+ tanjun/context/menu.py:101: note:          Optional[MenuCommand[Any, Any]]

steam.py (https://github.com/Gobot1234/steam.py)
+ steam/ext/csgo/models.py:178: note:      Superclass:
+ steam/ext/csgo/models.py:178: note:          def inventory(self, App[str | None], /, *, context_id: int | None = ..., language: Language | None = ...) -> Coroutine[Any, Any, Inventory[Item[ClientUser], ClientUser]]
+ steam/ext/csgo/models.py:178: note:      Subclass:
+ steam/ext/csgo/models.py:178: note:          @overload
+ steam/ext/csgo/models.py:178: note:          def inventory(self, app: Any, *, language: object = ...) -> Coroutine[Any, Any, Backpack]
+ steam/ext/csgo/models.py:178: note:          @overload
+ steam/ext/csgo/models.py:178: note:          def inventory(self, app: App[str | None], *, language: Language | None = ...) -> Coroutine[Any, Any, Inventory[Item[ClientUser], ClientUser]]