-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Comments
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 |
As far as I can tell, the same problem applies to |
@glyph Perhaps you could use I wonder if we could enable |
What I’m trying to do is to write a class decorator, |
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. |
@JukkaL 🙏 |
@JukkaL I think I found a workaround, leveraging the little white lie that 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 |
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
but these have to come after the class body and look somewhat uglier. |
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. |
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 I have also just realized that this breaks (mypy raises this error) even when you write a function that acts like 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 |
In some cases you may use 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. |
@JukkaL , Thanks. That's not enough, though. Mypy treats (frankly, it should) classes as abstract even when they don't specify Maybe it would be possible to use a similar trick with 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 |
Just to add another data point here:
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:
The behavior is if |
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 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 |
Maybe try:
|
@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):
I also tried using |
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 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]) |
@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. |
## 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>
To me, disabling the
|
I am creating a library for rest api clients with a bunch of code generation stuff. I take method signatures from 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
At the same time, PyCharm 2023.2.1 (Community Edition) understands that instances of |
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. |
Does the workaround suggested here work? #4717 (comment) |
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. |
The only way to fix this is to remove this misguided error entirely. My latest encounter with it involves an @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? |
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 ;-) |
I agree with that the bug is in |
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 :) |
@rafalkrupinski 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 def get(self, dependency_type: Type[T]) -> T: Project link: https://github.com/reagento/dishka |
We have a very similar situation in https://github.com/esss/oop-ext, which implements interfaces which are checked at runtime, and also subclass We have a function I believe there are many legitimate cases to pass an abstract type as parameter, not only to instantiate it, which |
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 I think the only viable option now is to push for changing the official spec. (Meanwhile, my code base gets filled with ugly |
This line seems wrong: def fun(cls: Type[Proto]) -> int:
return cls().meth() # OK It's not OK even if |
I think the spec document insists on enforcing LSP on constructors, and This is indeed clearly diverged from how the major type checkers are implemented. 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/
|
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
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
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
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
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
Edit: After a few days, I realized that we should not break PEP 544 as it's standardized for 7 years, Following is the original comment. I agree with @NeilGirdhar, that it's not reasonable to assume some behavior on "calling constructor on some type". And I want to provide another point of view that type itself doesn't assure there is a constructor either. Consider we have a build function def build(the_type: type[_T], fact: Callable[[], _T] | None) -> _T:
return the_type() if fact is None else fact()
def new_generator() -> Callable[[], int]:
return lambda: random.choice(range(100))
gen_good: Callable[[], int] = new_generator()
gen_bad: Callable[[], int] = build(Callable[[], int], new_generator) # here _T is Callable[[], int]
# Although this line passes `type-abstract`, it fails by `arg-type`, which may deserve discussion in some other thread. In this case, evaluate This use-case may seem a little weird, but we can make it like an ordinary class, def build(the_type: type[_T], fact: Callable[[], _T] | None) -> _T:
return the_type() if fact is None else fact()
class Generator(Protocol):
def __call__(self) -> int: ...
class RandomGenerator:
def __call__(self) -> int:
return random.choice(range(100))
gen_obj_good: Generator = RandomGenerator()
gen_obj_bad: Generator = build(Generator, RandomGenerator) # check fail by `type-abstract` The usage of |
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 ofsome_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):
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:
Unfortunately, MyPy complains about this as follows:
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:
isinstance
The text was updated successfully, but these errors were encountered: