Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Poor interactions with staticmethod and overload/union #15015

Open
ksunden opened this issue Apr 7, 2023 · 9 comments
Open

Poor interactions with staticmethod and overload/union #15015

ksunden opened this issue Apr 7, 2023 · 9 comments
Labels
bug mypy got something wrong topic-descriptors Properties, class vs. instance attributes

Comments

@ksunden
Copy link

ksunden commented Apr 7, 2023

Bug Report

Behavior changed in v1.2.0 regarding how staticmethods are handled when invoked by doing foo = staticmethod(bar) (i.e. not as the @decorator syntax)

A realworld example of this appeared in matplotlib, which is wrapping a third party method as a staticmethod of one of our classes (pyparsing)

Possibly related:

#14953 (most recent change to staticmethod handling)
#14902 (typeshed sync, mentions staticmethod
#7781 (original report of staticmethod behavior

A pure python example (and extension) is provided below.

To Reproduce

from typing import overload, Callable, Any
@overload
def foo(a: int) -> None:
    ...
    
@overload
def foo(a: str) -> None: ...

def foo(a) -> None:
    ...

def func_factory(spam) -> Callable[[], Any]| Callable[[int], Any]:
    return lambda: None
    
def qux(i: int) -> int:
    return i

class Foo():
    foo = staticmethod(foo)
    bar = staticmethod(func_factory(None))
    
    @overload
    @staticmethod
    def baz(a: str) -> str: ...
    @overload
    @staticmethod
    def baz(a: int) -> int: ...
    
    @staticmethod
    def baz(a):
        return a
        
    qux = staticmethod(qux)

reveal_type(foo)  # Correctly 'Overload(def (a: builtins.int), def (a: builtins.str))'
reveal_type(Foo.foo) # Should match above, actually 'def (*Any, **Any) -> Any' on v1.1.1 and "def (a: builtins.int)" on v1.2.0

reveal_type(func_factory(None)) # Correctly 'Union[def () -> Any, def (builtins.int) -> Any]'
reveal_type(Foo.bar) # Should match above, actually 'def (*Any, **Any) -> Any' on v1.1.1 and "def (*<nothing>, **<nothing>) -> Any" on v1.2.0

reveal_type(Foo.baz) # Correctly "Overload(def (a: builtins.str) -> builtins.str, def (a: builtins.int) -> builtins.int)"

reveal_type(qux)  # Correctly "def (i: builtins.int) -> builtins.int"
reveal_type(Foo.qux) # Should match above, actually 'def (*Any, **Any) -> builtins.int' on v1.1.1 and correct on v1.2.0

Foo.foo("a") # Errors on v1.2.0

https://mypy-play.net/?mypy=latest&python=3.10&gist=c50c0fb3a6c10fbea317a31453b40e29

Expected Behavior

I would expect that the overload (or union) would carry forward to the decorated static method.

Actual Behavior

Prior to v1.2.0 (tested explicitly with v1.1.1:
All staticmethods constructed without @decorator syntax would simply get def (*Any, **Any) -> Any, which was at least not an error, though was throwing away type information.

In the case of a single non-overloaded function defined outside of the class, it gets the return type but not the parameter type.

In v1.2.0:

In the case of an overloaded method defined outside of the class, only the first overloaded definition is kept, resulting in incorrect errors for alternate signatures.

In the case of a Union type (such as from a factory function), an error is thrown on the call to staticmethod, even if all branches of the union satisfy the type staticmethod expects.
This was the actual case I saw with matplotlib.

In the case of a single non-overloaded function or an overload defined in the class (with @staticmethod on all overload branches) v1.2.0 behaves as expected.

main.py:21: error: Argument 1 to "staticmethod" has incompatible type "Union[Callable[[], Any], Callable[[int], Any]]"; expected "Callable[[VarArg(<nothing>), KwArg(<nothing>)], Any]"  [arg-type]
main.py:36: note: Revealed type is "Overload(def (a: builtins.int), def (a: builtins.str))"
main.py:37: note: Revealed type is "def (a: builtins.int)"
main.py:39: note: Revealed type is "Union[def () -> Any, def (builtins.int) -> Any]"
main.py:40: note: Revealed type is "def (*<nothing>, **<nothing>) -> Any"
main.py:42: note: Revealed type is "Overload(def (a: builtins.str) -> builtins.str, def (a: builtins.int) -> builtins.int)"
main.py:44: note: Revealed type is "def (i: builtins.int) -> builtins.int"
main.py:45: note: Revealed type is "def (i: builtins.int) -> builtins.int"
main.py:47: error: Argument 1 has incompatible type "str"; expected "int"  [arg-type]
Found 2 errors in 1 file (checked 1 source file)

Your Environment

  • Mypy version used: v1.2.0 (compared to v1.1.1)
  • Mypy command-line flags: N/A
  • Mypy configuration options from mypy.ini (and other config files):N/A
  • Python version used: 3.11
@ksunden ksunden added the bug mypy got something wrong label Apr 7, 2023
@hoodmane
Copy link

hoodmane commented Apr 7, 2023

I was just going to report the same bug. I think the simplest reproduction is:

def f(a: int) -> int:
    return 7

@staticmethod
def g(a: int) -> int:
    return 7


reveal_type(staticmethod(f)) # Revealed type is "builtins.staticmethod[builtins.int]"
reveal_type(g) # Revealed type is "def (a: builtins.int) -> builtins.int"

@ksunden
Copy link
Author

ksunden commented Apr 7, 2023

@hoodmane That particular presentation is actually fixed on master/v1.2.0, I believe.

It is roughly equivalent to the qux case in my original report

main.py:57: note: Revealed type is "builtins.staticmethod[[a: builtins.int], builtins.int]"
main.py:58: note: Revealed type is "def (a: builtins.int) -> builtins.int"

It still has the wrapper shown in the revealed type, but it retains the type info for the proper signature. But staticmethod doesn't make semantic sense outside of a class, so that is probably where the wrapper is coming from. (mypy also errors on the staticmethod of g in your example because it is outside of a class)

The overload/union signatures are an extra wrinkle that isn't quite right still

@hoodmane
Copy link

hoodmane commented Apr 7, 2023

Well the behavior is the same with:

def f(a: int) -> int:
    return 7

class T:
   f = staticmethod(f)

reveal_type(T.f)

which is where it's actually useful. But even if mypy is mad about the use of staticmethod it should be able to understand that it doesn't change the type of the argument (by very much).

@hoodmane
Copy link

hoodmane commented Apr 7, 2023

Ah but I see it most definitely does work on mypy v1.2.0. I was testing against v1.1.1. Thanks!

@AlexWaygood AlexWaygood added the topic-descriptors Properties, class vs. instance attributes label Apr 11, 2023
@ksunden
Copy link
Author

ksunden commented Apr 21, 2023

It appears as though the overload portions of this report have been addressed on master. The union portions do seem to remain (unfortunately that is what is causing problems for matplotlib)

@b-kamphorst
Copy link

b-kamphorst commented Apr 26, 2023

I bump into the same issue. Furthermore, I think you can add 'generic' to the list if incompatibilities -- if foo is annotated with a: T, where T is a TypeVar, then reveal_type(Foo.foo) reveals Revealed type is "def [T] (a: Foo) -> Foo".

(Tested on mypy-1.4.0+dev.09d469f3ea845c682d9be4af6aba889cb5852396.)

@b-kamphorst
Copy link

This might be a special case of #15737, staticmethod is annotated with paramspec as well.

@ksunden
Copy link
Author

ksunden commented Apr 1, 2024

Update here, things have improved to a degree but not fully resolved as of mypy 1.9.0:

  • Foo.foo (staticmethod of an overloaded external function) still incorrect, matches behavior of v1.2.0, where it keeps only the first prong of the overload
  • Foo.bar (staticmethod of a union of Callables) has reverted to prior behavior like in v1.1.1, i.e. loses all type information, but does not error
  • Foo.baz (staticmethod overloaded inline) remains correct, and has always been so (well, since at least 1.1.1)
  • Foo.qux (staticmethod of non-overloaded external function) remains correct (as it was in 1.2.0)

@Jamim
Copy link

Jamim commented May 9, 2024

I'm still having this issue with mypy 1.10.0.

Here is a snippet to reproduce it.
from typing import Callable


class Base:
    def __init__(self, test: Callable[..., None] | None):
        self.test = test


def test() -> None:
    pass


class Good(Base):
    test = staticmethod(test)


class Bad(Base):
    @staticmethod
    def test() -> None:
        pass
$ mypy --strict test.py
test.py:19: error: Signature of "test" incompatible with supertype "Base"  [override]
test.py:19: note:      Superclass:
test.py:19: note:          Callable[..., None] | None
test.py:19: note:      Subclass:
test.py:19: note:          @staticmethod
test.py:19: note:          def test() -> None
Found 1 error in 1 file (checked 1 source file)

It forces me to uglify some child classes in a real application.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-descriptors Properties, class vs. instance attributes
Projects
None yet
Development

No branches or pull requests

5 participants