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

abstract class shouldn't be considered Callable #12361

Open
beauxq opened this issue Mar 16, 2022 · 9 comments
Open

abstract class shouldn't be considered Callable #12361

beauxq opened this issue Mar 16, 2022 · 9 comments
Labels
bug mypy got something wrong

Comments

@beauxq
Copy link

beauxq commented Mar 16, 2022

Bug Report

An abstract class fits the typing for Callable.

To Reproduce

import abc
from typing import Callable


class A(abc.ABC):
    @abc.abstractmethod
    def m(self) -> None:
        ...


class C(A):
    def m(self) -> None:
        pass


def f() -> A:
    return C()


def t(x: Callable[[], A]) -> None:
    x()


def main() -> None:
    t(f)
    t(C)
    t(A)  # should give typing error - A is not callable because it can't be instantiated
    A()  # mypy correctly reports that A can't be instantiated


if __name__ == "__main__":
    main()

Expected Behavior

mypy should report a typing error if an abstract class is put where a Callable should go

Actual Behavior

mypy doesn't report any error if an abstract class is put where a Callable should go

Your Environment

  • Mypy version used: 0.910
  • Python version used: 3.8.10
@beauxq beauxq added the bug mypy got something wrong label Mar 16, 2022
@mixed-source
Copy link
Contributor

The built-in callable function returns true on the abstract class. I guess the typing error is not raised due to ABC appears to be callable but it's not callable as it can't be instantiated.

@agronholm
Copy link

I strongly disagree with this. AnyIO has a number of abstract classes where their __new__() methods return an instance of a subclass.

@beauxq
Copy link
Author

beauxq commented Dec 19, 2022

I strongly disagree with this. AnyIO has a number of abstract classes where their __new__() methods return an instance of a subclass.

Can you provide an example to show that this is a pattern worth supporting?

If a Python API user calls a class, they expect to get an instance of that specific class, not an instance of a subclass.

@agronholm
Copy link

AnyIO is an async "meta-framework" that delegates actual I/O to underlying frameworks like asyncio or Trio. Therefore it has classes like Lock and Semaphore which, when instantiated, look up the current event loop implementation (using sniffio) and then use the appropriate back-end to instantiate the actual implementation (which would be a subclass of this abstract class). Implementing this in __new__() is far better than the alternative of having separate factory methods like create_lock() which AnyIO had in the past.

@beauxq
Copy link
Author

beauxq commented Dec 20, 2022

Implementing this in __new__() is far better than the alternative of having separate factory methods like create_lock()

I disagree with this. Having a separate factory method like create_lock() would be better.
The reason it would be better is that if a Python API user calls a class, they expect to get an instance of that specific class, not an instance of a subclass.

@agronholm
Copy link

That ship has sailed, and nobody has complained about this design change. It makes it much easier to shift from asyncio to AnyIO when these classes essentially work the same way in both libraries. I dislike that MyPy enforces certain design choices on its users, without them even being mandated by any PEP whatsoever.

@edaqa-uncountable
Copy link

If you have a __new__ method, couldn't mypy infer that it is indeed callable, thus not causing a problem with the AnyIO code? At the least it should probably emit a warning if an abstract class has a __new__ method, since that creates an unusual contract that it probably can't track.

Otherwise, I think the philosophy is that types are meant to enforce the expected behaviour, and a __new__ method is generally expected to return the class being instantiated. If a different type is returned, then the type checker is already incorrect for that class, since it will infer the wrong type. If __new__ is returning the correct type, and it's usable, then one could argue that it's the abstract aspect of the class that isn't actually valid. That is, if Lock allows Lock() and returns a Lock type, it's hard to consider it an abstract class.

I think it'd be fair, in this respect, to require an alternate notation to mark this kind of special behaviour. I'd also assume that requiring this special notation is the lesser evil; being able to assign an abstract class to a Callable will result in runtime failure.

@agronholm
Copy link

Otherwise, I think the philosophy is that types are meant to enforce the expected behaviour, and a new method is generally expected to return the class being instantiated.

Python has, as long as I remember, allowed __new__() to return an arbitrary object, even one not related to the original class at all, in which case the __init__() from the original class won't be called on the return value.

Therefore, if a class with a __new__() method is being passed as a Callable, then that __new__() method is the one that should be checked against that Callable spec. Would this not solve the issue?

@edaqa-uncountable
Copy link

Therefore, if a class with a __new__() method is being passed as a Callable, then that __new__() method is the one that should be checked against that Callable spec. Would this not solve the issue?

Yes, that sounds right. It's not about the class, but about the __new__ method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong
Projects
None yet
Development

No branches or pull requests

4 participants