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

Typing #189

Open
bobthemighty opened this issue Oct 19, 2024 · 4 comments
Open

Typing #189

bobthemighty opened this issue Oct 19, 2024 · 4 comments

Comments

@bobthemighty
Copy link
Owner

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

container.register(SomeService, SomeImpl)
container.resolve(SomeService)

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 while resolve(k: any) -> any.

The situation for registrations is likely to be harder.

@jonaslb
Copy link

jonaslb commented Nov 6, 2024

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 resolve(k: str) -> Any.

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 infer_variance by default when using the new generic syntax class X[T] (instead of class X(Generic[T])), so question is if it is even possible to obtain variance at runtime.

@jonaslb
Copy link

jonaslb commented Nov 7, 2024

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. Sender[A] was a registered service, that a request to resolve Sender[B] would use that registered service if B was a subclass of A. But it could just be considered either out of scope or maybe even intentional the way it is.

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)?

@bobthemighty
Copy link
Owner Author

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.

@jonaslb
Copy link

jonaslb commented Nov 13, 2024

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.
I think probably the API ought to be changed to seem cleaner with types added. I.e. maybe it would make sense to have different resolves for types, instances of types and maybe things stored under string keys should be disallowed. That's a much bigger change but it would make typing a lot simpler.

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

No branches or pull requests

2 participants