-
Notifications
You must be signed in to change notification settings - Fork 15
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
Typing #189
Comments
Thanks for the library :) I think accepting this as a breaking change would be best. I'm not sure how "actually breaking" it would be since I guess the library is mostly used like that already. An override could perhaps be kept around for just the usecase of What I wanted to point out that there are probably some particular challenges around generics here. For example, class A:
pass
class B(A):
pass
class C(B):
pass
T = TypeVar("T") # invariant
T_co = TypeVar("T_co", covariant=True)
T_contra = TypeVar("T_contra", contravariant=True)
class X(Generic[T]):
pass
class XCo(Generic[T_co]):
pass
class XContra(Generic[T_contra]):
pass
cont = punq.Container()
# Invariant case, seems ok?
x = X()
cont.register(X[B], instance=x)
cont.resolve(X[B]) is x # true
# cont.resolve(X[C]) # ok: will error (not found)
# cont.resolve(X[A]) # ok: will error (not found)
# Covariant / contravariant not as expected
xco = XCo()
xcontra = XContra()
cont.register(XCo[B], instance=xco)
cont.register(XContra[B], instance=xcontra)
cont.resolve(XCo[A]) is xco # not ok: will error (but xco is XCo[B] so it is assignable to XCo[A])
cont.resolve(XContra[C]) is xcontra # not ok: will error (but xcontra is XContra[B] so it is assignable to XContra[C]) And I'm not sure how to overcome that, it seems like it would require a lot of type related logic. I don't think it's impossible though. Edit: And also, type variables have |
It might actually just be as intended, since it also doesn't work with plain inheritance. So maybe this is just a nothingburger. I guess I had just thought that if e.g. I might give this a go if you would be accepting of a PR on the matter (i.e. just annotations, not enabling what I had described above)? |
I would be accepting! I think it would be deferred to a 1.0 because I suspect it does break some use cases, but I'd love to collaborate, so I'm not battling pyright on my lonesome. |
I'm having sort of second thoughts about hopping on this at least short term, and don't want to block anyone else taking it up. |
In the years since I wrote this lib, as a proof of concept, people have
A) started using it in earnest, much to my surprise
B) adopted type hints as a best practice for Python development
It would be nice if the container supported typing, at least for registration, so that
is known to return SomeService. The challenge here is that the key for the container isn't restricted in any way, so this is likely a breaking change, unless we can get fancy with Pyright so that
resolve[TService](k: Type[TService]) -> TService
whileresolve(k: any) -> any
.The situation for registrations is likely to be harder.
The text was updated successfully, but these errors were encountered: