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

Use case for typing.Type with abstract types #4717

Open
erikwright opened this issue Mar 11, 2018 · 74 comments
Open

Use case for typing.Type with abstract types #4717

erikwright opened this issue Mar 11, 2018 · 74 comments
Labels

Comments

@erikwright
Copy link

In #2169 and #1843 there was discussion about using Type[some_abc] and how it should be allowed in a function signature, but that the call-site was expected to pass a concrete subclass of some_abc. There is an implicit assumption that the objective of a function taking such a type is, ultimately, to instantiate the type.

@gvanrossum said, in #2169 (comment):

But maybe we need a check that you call this thing with a concrete subclass;
and for that we would need some additional notation (unless maybe we could
just say that whenever there's an argument of Type[A] where A is abstract,
that the argument must be a concrete subclass. But for that we'd need some
experiment to see if there's much real-world code that passes abstract
classes around. (If there is, we'd need to have a way to indicate the need
in the signature.)

I have such a use case.

I have a sequence of observers supplied by clients of my library, to which I want to dispatch events according to the abstract base class(es) that each implements. I tried to do this as follows:

import abc
import typing

class FooObserver(metaclass=abc.ABCMeta):
    """Receives Foo events."""

    @abc.abstractmethod
    def on_foo(self, count: int) -> None:
        raise NotImplementedError()

class BarObserver(metaclass=abc.ABCMeta):
    """Receives Bar events."""

    @abc.abstractmethod
    def on_bar(self, status: str) -> None:
        raise NotImplementedError()

class Engine:
    def __init__(self, observers: typing.Sequence[typing.Any]) -> None:
        self.__all_observers = observers

    def do_bar(self, succeed: bool) -> None:
        status = 'ok' if succeed else 'problem'
        for bar_observer in self.__observers(BarObserver):
            bar_observer.on_bar(status)

    def do_foo(self, elements: typing.Sequence[typing.Any]) -> None:
        count = len(elements)
        for foo_observer in self.__observers(FooObserver):
            foo_observer.on_foo(count)

    __OBSERVER_TYPE = typing.TypeVar('__OBSERVER_TYPE')

    def __observers(
            self,
            observer_type: typing.Type['__OBSERVER_TYPE']
    ) -> typing.Sequence['__OBSERVER_TYPE']:
        return [observer for observer in self.__all_observers
                if isinstance(observer, observer_type)]

Unfortunately, MyPy complains about this as follows:

/Users/erikwright/abc_typing.py:24: error: Only concrete class can be given where "Type[BarObserver]" is expected
/Users/erikwright/abc_typing.py:29: error: Only concrete class can be given where "Type[FooObserver]" is expected

Given that (AFAICT) the decision was made to not attempt to verify that the runtime type supports any specific constructor signature I'm wondering why there is nonetheless an expectation that the runtime type is constructable at all. In my case, the entire purpose of typing here is:

  1. Require you to actually pass a type, which means I can use it in isinstance
  2. Allow me to specify the return type of the method in terms of the supplied type.
@ilevkivskyi
Copy link
Member

The point is that it is too hard to track which classes are instantiated in the body of a given function, and which are not. If we would have such tracking, then we could allow calling functions with abstract classes at particular positions.

Taking into account that such tracking would take some effort, and that during the year of current behaviour, this is a first such request, I would recommend just using # type: ignore. Even if this will be implemented at some point, this is quite low priority.

@glyph
Copy link

glyph commented Jan 15, 2019

As far as I can tell, the same problem applies to Protocol as well. Is there any way to have a TypeVar that references anything abstract?

@JukkaL
Copy link
Collaborator

JukkaL commented Jan 15, 2019

@glyph Perhaps you could use # type: ignore to silence the error as suggested above? It's clearly not optimal, though. Can you give more information about your use case?

I wonder if we could enable Type[x] with abstract and protocol types but disallow creating an instance (i.e they wouldn't be callable). They could still be used for things like isinstance checks.

@glyph
Copy link

glyph commented Jan 15, 2019

What I’m trying to do is to write a class decorator, @should_implement(SomeProtocol) which type-checks the decorated class to ensure it complies with the given protocol, so ignoring the error would obviate the whole point ;-).

@JukkaL
Copy link
Collaborator

JukkaL commented Jan 15, 2019

Makes sense, though I'm not sure if lifting the restriction would be sufficient to allow the decorator to be checked statically. Right now a runtime check is probably the best you can do with that syntax, at least without a plugin. For a static check you could use a dummy assignment (which is not very pretty).

Increasing priority to normal since I think that the current approach is too restrictive. I'm not sure what's the best way to move forward, though.

@glyph
Copy link

glyph commented Jan 15, 2019

@JukkaL 🙏

@glyph
Copy link

glyph commented Jan 16, 2019

@JukkaL I think I found a workaround, leveraging the little white lie that Protocols without constructors are callables that return themselves:

from typing import Callable, Type, TypeVar
from typing_extensions import Protocol

class AProtocol(Protocol):
    x: int


protocol = TypeVar("protocol")
def adherent(c: Callable[[], protocol]) -> Callable[[Type[protocol]], Type[protocol]]:
    def decor(input: Type[protocol]) -> Type[protocol]:
        return input
    return decor


@adherent(AProtocol)            # No error; Yes is the expected shape
class Yes(object):
    x: int
    other: str

y = Yes()
y.x
y.other


@adherent(AProtocol)            # We get an error here, as desired
class No(object):
    y: int

@glyph
Copy link

glyph commented Aug 6, 2019

I should note that there's a big problem with my workaround; you can only apply the decorator once, and then it breaks down. There's a variant here where you can abuse a Generic instead, and then write

Adherent[AProtocol](Yes)
Adherent[AProtocol](No)

but these have to come after the class body and look somewhat uglier.

@ilevkivskyi
Copy link
Member

I'm not sure what's the best way to move forward, though.

A possible ad-hoc solution (which may be not so bad), is to just remove this check for class decorators, because it also causes troubles for dataclasses, see #5374 that has 12 upvotes.

@petr-motejlek
Copy link

petr-motejlek commented Feb 1, 2020

This "Only concrete class can be given" error also seems impossible to overcome for code that is supposed to accept an abstract class and return some instance of that class, even if it would have to create the class right then and there using some fancy mechanism like type(a, b, c). Think unittest.Mock and similar dummy object factories.

I have also just realized that this breaks (mypy raises this error) even when you write a function that acts like isinstance(...) with the provided class. Makes you wonder how isinstance is actually typed in typeshed (gonna check that now).

from abc import ABC
from typing import TypeVar, Type

T = TypeVar('T')

class Abstract(ABC):
  pass

def isthisaninstance(this, type_: Type[T]) -> bool:
  return isinstance(this, type_)

isthisaninstance("", Abstract)   # type: ignore :(

Is there any way to overcome this (other than to # type: ignore all function/method calls?

@JukkaL
Copy link
Collaborator

JukkaL commented Feb 4, 2020

Is there any way to overcome this

In some cases you may use if TYPE_CHECKING: to conditionally define the base class so that mypy sees the base class as object while at runtime it will be ABC:

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    Base = object
else:
    Base = ABC

class Abstract(Base):
    pass

You'll lose any ABC checking by mypy, however.

@petr-motejlek
Copy link

petr-motejlek commented Feb 5, 2020

@JukkaL ,

Thanks. That's not enough, though. Mypy treats (frankly, it should) classes as abstract even when they don't specify ABC as their parent. It's enough for them to have @abstractmethods on them for example.

Maybe it would be possible to use a similar trick with TYPE_CHECKING to override even @abstractmethod (set it to some no-op decorator), but that's really pushing it :D.

Today, we have dealt with this issue in our code in a way where we use @OverRide to allow our code to be called with an abstract class, and possibly even return its instance, however, the user has to specify the return type for the variable where they are storing it. Which isn't that bad.

from typing import Type, TypeVar, overload

T = TypeVar('T')

@overload
def do(type_: Type[T], ...) -> T:
    pass


@overload
def do(type_: type, ...) -> T:
    pass


def do(type_: Type[T], ...) -> T:
    """
    Do ...
    """

from abc import ABC
class Abstract(ABC):
    pass

var: Abstract = do(Abstract, ...)

I've had to redact some of the bits, but this works, unless somebody takes out the type annotation for var. It's a bit wonky, because of course, if you lie to your face and mistype the type annotation for var, do will likely give you something different. But I think it's better than if TYPE_CHECKING or # type: ignore :D. Especially since we actually use this particular function with concrete classes.

@jstasiak
Copy link
Contributor

Just to add another data point here:

This "Only concrete class can be given" error also seems impossible to overcome for code that is supposed to accept an abstract class and return some instance of that class, even if it would have to create the class right then and there using some fancy mechanism like type(a, b, c). Think unittest.Mock and similar dummy object factories.

This is what happens in Injector (a dependency injection framework, I imagine most similar frameworks will have this pattern somewhere). The Injector class' get() method is typed like this:

get(interface: Type[T], ...) -> T

The behavior is if T is abstract an instance of something non-abstract will be provided (if bound earlier) or a runtime error occurs, and from Injector's point of view it'd be cool if this type checked: python-injector/injector#143 :)

@bmerry
Copy link
Contributor

bmerry commented Jun 12, 2020

I've run into this problem too: I have a function in which takes a (possibly abstract) type and returns an instance of that type; but in my case it does it by calling a class method on the type. What I'd ideally like to be able to do is declare a protocol that inherits from Type[_T] e.g.

class _Factory(Protocol[_T], Type[_T]):
   def from_file(cls, file: io.IOBase) -> _T

to indicate that the argument must be a class which provides a from_file class method returning an instance of itself (_T being covariant). Unfortunately when I try this I get 'error: Invalid base class "Type"'; and if I take the Type base class out then I get lots of other errors referring to "" which I'm guessing is because there is no way for mypy to deduce which _T to use in the protocol.

If something like this was possible (and I don't understand type theory nearly well enough to say whether it makes sense), it would make it practical to express concepts like "subclass of X which is instantiatable", "subclass of X which has this __init__ signature" or "subclass for X for which this particular abstract class method has an implementation" (my use case).

@JelleZijlstra
Copy link
Member

Maybe try:

class _Factory(Protocol[_T]):
   def from_file(cls: Type[_T], file: io.IOBase) -> _T

@bmerry
Copy link
Contributor

bmerry commented Jun 12, 2020

@JelleZijlstra thanks. I tried something like that and couldn't get it to work. Here's an example with your suggestion:

#!/usr/bin/env python3

from abc import ABC, abstractmethod
from typing import Type, TypeVar
from typing_extensions import Protocol

_T_co = TypeVar('_T_co', covariant=True, bound='Base')
_T = TypeVar('_T', bound='Base')

class Base(ABC):
    @abstractmethod
    def foo(self) -> None: ...

    @classmethod
    def from_file(cls: Type[_T]) -> _T: ...

class Derived(Base):
    def foo(self) -> None: ...

class _Factory(Protocol[_T_co]):
    def from_file(cls: Type[_T_co]) -> _T_co: ...

def make_thing(cls: _Factory[_T]) -> _T: ...

make_thing(Base)

It gives these errors (mypy 0.780):

type_proto.py:21: error: The erased type of self "Type[type_proto.Base]" is not a supertype of its class "type_proto._Factory[_T_co`1]"
type_proto.py:25: error: Argument 1 to "make_thing" has incompatible type "Type[Base]"; expected "_Factory[<nothing>]"

I also tried using _T_co throughout instead of just _T since I don't think I really understand how variance interacts with generic-self, but it didn't work any better.

@Purg
Copy link

Purg commented Oct 27, 2020

I'm also hitting up against this issue. I have a similar case where I have a plugin system that utilizes sub-type filtering. The following fails with the same error when invoked where interface_type is of an abstract class:

import abc
from typing import Collection, Set, Type, TypeVar

T = TypeVar("T")

def filter_plugin_types(interface_type: Type[T], candidate_pool: Collection[Type]) -> Set[Type[T]]:
    """Return a subset of candidate_pool based on the given interface_type."""
    ...

class Base(abc.ABC):
    @abc.abstractmethod
    def foo(self) -> None: ...

class Derived(Base):
    def foo(self) -> None: ...

type_set = filter_plugin_types(Base, [object, Derived])

@pauleveritt
Copy link

@Purg As I'm also working on a plugin system, would be interested in hearing more about what you're doing.

@Purg
Copy link

Purg commented Oct 29, 2020

@Purg As I'm also working on a plugin system, would be interested in hearing more about what you're doing.

Hi @pauleveritt, thanks for your interest! I sent a message to your email you have listed.

@mjog
Copy link

mjog commented May 14, 2023

My use case is using ABCs in a dict:

class SomeABC(...): ...
class DerivedABC1(SomeABC): ...
class DerivedABC2(SomeABC): ...

map: dict[type[SomeABC], ...] = {
    DerivedABC1: ...,
    DerivedABC2: ...
}
print(map[DerivedABC1]) # <-- mypy gives false positive here

Using dicts for lookup and dispatch is a fundamental idiom in Python. This should not produce an error by default.

nicoddemus added a commit to ESSS/oop-ext that referenced this issue May 16, 2023
However due to python/mypy#4717, this breaks
IsImplemenation with errors like this one:

   Only concrete class can be given where "Type[_InterfM3]" is expected  [type-abstract]

Which is unfortunate.
levand added a commit to chroma-core/chroma that referenced this issue Jun 2, 2023
## Description of changes

All component instances now inherit from a base `Component` class
defining `start` and `stop` methods, and to track their dependencies
throughout the system.

The `System` class has been updated to start and stop all components in
dependency order.

All these material changes are in `chromadb/config.py`

This was necessary to properly implement testing in the new
architecture. The new version of the system has more stateful components
with (e.g.) active subscriptions. It is necessary to provide an explicit
mechanism to shut down the whole stack or else consumers could continue
operating unexpectedly in the background and not be garbage collected.

Most of the changes in this PR are to fix existing type signature
errors. All components of the system are now subclasses of
`EnforceOverrides` which ensures that all signatures match at runtime,
and which operates independently from the type checker (so the `#type:
ignore` is not sufficient to fix it.)

This PR also disables the `type-abstract` MyPy error code. In my opinion
this is an incorrect type rule although there is
[python/mypy#4717 discussion) on the
topic.

## Test plan

Unit + integration tests updated and passing.

## Documentation Changes

No changes to user-facing APIs.

---------

Co-authored-by: hammadb <hammad@trychroma.com>
@ippeiukai
Copy link

ippeiukai commented Jul 24, 2023

To me, disabling the type-abstract error code does not solve following problems:

  1. When using @overload, non-concrete type objects do not match type[T] with or without disabling type-abstract. This results in other errors than type-abstract such as assignment or attr-defined being reported later in the code.
    See cases in Protocol type object is not resolved correctly with overload #15666 .
  2. Assigning Protocol type object Proto to variables annotated with type[Proto] is actually forbidden in a special rule in PEP 544. Unlike abstract classes, this is enforced by other type checkers like pyright. Because protocols and abstract classes are treated alike in mypy, disabling type-abstract errors stops reporting cases deemed invalid in PEP 544, and in turn results in incompatibility with other type checkers.

@Tishka17
Copy link

Tishka17 commented Oct 6, 2023

I am creating a library for rest api clients with a bunch of code generation stuff. I take method signatures from Protocol and generate their bodies using some meta information declared additionally. I decided to use class decorators for this feature.

Actually the code is expected to be like this:

_ProtoT = TypeVar("_ProtoT")
def with_protocol(proto: Type[_ProtoT]) -> Callable[[type], Type[_ProtoT]]:
    def decorator(cls: type) -> Type[_ProtoT]:
        # some stuff here
        return type(
            cls.__name__ + "_generated",
            (cls, proto),
            generated_methods,
        )

    return decorator


class Client(Protocol):
   @abstractmethod
   def get_item(self, item_id) -> Item:
      pass


@with_protocol(Client):
class RealClient(SomeBase):
    get_item = get("url here")  # just storing meta information

What should I set as annotations for function with_protocol? Currently I get an error

Only concrete class can be given where "Type[Client]" is expected

At the same time, PyCharm 2023.2.1 (Community Edition) understands that instances of RealClient_generated are implementing Client protocol

@NeilGirdhar
Copy link
Contributor

What should I set as annotations for function with_protocol?

This may not be a popular answer, but personally I think you should disable that MyPy error since I personally don't think it's a good error.

@glyph
Copy link

glyph commented Oct 11, 2023

What should I set as annotations for function with_protocol? Currently I get an error

Does the workaround suggested here work? #4717 (comment)

@glyph
Copy link

glyph commented Oct 11, 2023

What should I set as annotations for function with_protocol?

This may not be a popular answer, but personally I think you should disable that MyPy error since I personally don't think it's a good error.

Disabling the error doesn't help for library authors, because you have to tell all your downstream users to disable it as well — they may not want to, and also they lose the benefit of type checking because the abstract type not being accepted breaks the definition of the decorated class as well.

@agronholm
Copy link

agronholm commented Oct 11, 2023

The only way to fix this is to remove this misguided error entirely. My latest encounter with it involves an attrs class where I had a field that checks if the value matches an abstract class:

@attrs.define(kw_only=True)
class Schedule:
    ...
    trigger: Trigger = attrs.field(
        eq=False,
        order=False,
        validator=instance_of(Trigger),  # type: ignore[type-abstract]
    )

As you can see above, I had to silence the error. But is somebody seriously telling me that what I'm doing here is wrong?

@glyph
Copy link

glyph commented Oct 11, 2023

is somebody seriously telling me that what I'm doing here is wrong?

It is certainly not "wrong" in an abstract sense of semantic correctness — the bug is clearly in Mypy here — but it might not be useful, particularly if you're writing a library. It might be nice if Attrs provided a workaround. But there's a tradeoff with maintenance effort and it might be fine for Attrs and similar libraries to just wait for the bug to be fixed. To truly answer your question we'd need to begin with a robust meta-ethics of open source ;-)

@gh-andre
Copy link

I agree with that the bug is in mypy, which assumes that whatever is passed in as Type[T] will result in a constructor call and reports this error. Quite often such types are used to call their static/class methods or obtain additional information, such as calling get_origin or get_type_hints, etc.

@rafalkrupinski
Copy link

rafalkrupinski commented Jan 24, 2024

I can't see any way to disable this check globally...

def find_annotations(
        user_type: type, annotation_type: ty.Type[T], ) -> ty.Sequence[T]:
    if user_type is inspect.Signature.empty:
        return ()
    return [anno for anno in user_type.__metadata__ if isinstance(anno, annotation_type)]  # type: ignore[attr-defined]

Each call with an abstract class for a parameter needs to be commented with an ignore :/

@Tishka17 Is your project publicly available? I'm making one too :)

@Tishka17
Copy link

@rafalkrupinski
My previous message was about https://github.com/reagento/dataclass-rest/ and I decided not to implement such Protocol inheritance.

Anyway I've got on more case: IoC-container. The point is that user registers different factories producing objects of certain types. And then, when you call container.get(SomeProtocol) it will find appropriate implementation and create it. The question is what is the signature of .get? Expectedly (accroding to this thread) this is not working

def get(self, dependency_type: Type[T]) -> T:

Project link: https://github.com/reagento/dishka

@nicoddemus
Copy link

Anyway I've got on more case: IoC-container.

We have a very similar situation in https://github.com/esss/oop-ext, which implements interfaces which are checked at runtime, and also subclass Protocol for static checking.

We have a function GetProxy(obj, interface_class), where the user can pass an object which implements the given interface, and it returns a proxy object with only the methods defined in the interface. Because interface_class is an abstract type, we get the mypy errors described in this thread, so we gave up on typing GetProxy appropriately.

I believe there are many legitimate cases to pass an abstract type as parameter, not only to instantiate it, which mypy currently assumes every function method that receives an abstract type will attempt to do.

@ippeiukai
Copy link

ippeiukai commented Mar 5, 2024

Sadly, the updated typing spec now clearly states that a non-concrete type object (an ABC or a protocol class) is not assignable to a variable that is explicitly typed typing.Type.
https://github.com/python/typing/blob/7117775f5465c6705bb753bd639e6255af380386/docs/spec/protocol.rst#type-and-class-objects-vs-protocols

I think the only viable option now is to push for changing the official spec.
Perhaps TypeForm proposal would be a good alternative?

(Meanwhile, my code base gets filled with ugly inject.instance(cast(type[SomeAbstractClass], SomeAbstractClass)) pattern which is needed to use non-concrete type objects as DI token...)

@NeilGirdhar
Copy link
Contributor

NeilGirdhar commented Mar 5, 2024

This line seems wrong:

def fun(cls: Type[Proto]) -> int:
    return cls().meth() # OK

It's not OK even if type[Proto] is concrete—there's no guarantee that there exists a constructor with that signature, and there's no way for a type checker to check that unless the class is also final.

@ippeiukai
Copy link

ippeiukai commented Mar 5, 2024

This line seems wrong:

def fun(cls: Type[Proto]) -> int:
    return cls().meth() # OK

It's not OK even if type[Proto] is concrete—there's no guarantee that there exists a constructor with that signature,

I think the spec document insists on enforcing LSP on constructors, and Proto not specifying one means Type[Proto] is supposed to accept only types with constructor that can be called without args.

This is indeed clearly diverged from how the major type checkers are implemented.
python/typing#1305

The above example being given as “the main reason” for variables and parameters annotated with Type[Proto] to accept only concrete (non-protocol) subtypes, might there be a space to propose spec changes on the whole thing? I wonder how far unsoundness argument can stretch…🤔

from https://peps.python.org/pep-0729/

Proposed changes to the specification, including PEPs, should generally be accompanied by the following:

  • Buy-in from type checker maintainers to confirm that the change can be implemented and maintained within their type checkers.
  • For changes to existing features, a survey of the behavior of existing type checkers. If existing type checkers behave roughly similarly, that is evidence that their shared behavior should be made part of the specification.
  • Changes to the conformance test suite that demonstrate the specified behavior.

AledWatkins added a commit to AledWatkins/squash-bot that referenced this issue Apr 6, 2024
Mypy can't understand that, even though this returns an instance of an
abstract base class, the instances will always be subclasses of the
base class and therefore they will have their methods defined.

python/mypy#4717
AledWatkins added a commit to AledWatkins/squash-bot that referenced this issue Apr 6, 2024
Mypy can't understand that, even though this returns an instance of an
abstract base class, the instances will always be subclasses of the
base class and therefore they will have their methods defined.

python/mypy#4717
AledWatkins added a commit to AledWatkins/squash-bot that referenced this issue Apr 6, 2024
Mypy can't understand that, even though this returns an instance of an
abstract base class, the instances will always be subclasses of the
base class and therefore they will have their methods defined.

python/mypy#4717
AledWatkins added a commit to AledWatkins/squash-bot that referenced this issue Apr 6, 2024
Mypy can't understand that, even though this returns an instance of an
abstract base class, the instances will always be subclasses of the
base class and therefore they will have their methods defined.

python/mypy#4717
AledWatkins added a commit to AledWatkins/squash-bot that referenced this issue Apr 6, 2024
Mypy can't understand that, even though this returns an instance of an
abstract base class, the instances will always be subclasses of the
base class and therefore they will have their methods defined.

python/mypy#4717
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests