Conversation
This comment has been minimized.
This comment has been minimized.
punq is an IOC (Inversion of Control) Container for Python 3.8+
This comment has been minimized.
This comment has been minimized.
stubs/punq/punq/__init__.pyi
Outdated
| service: type[_T] | str | ||
| scope: Scope | ||
| builder: Callable[[], _T] | ||
| needs: dict[str, object] |
There was a problem hiding this comment.
Due to the invariance of dict, using object as value type is often problematic. For example, the following won't type check with mypy:
x = {"": "a"}
y: dict[str, object] = x # incompatible assignmentsThe usual pattern here is to use Any.
stubs/punq/punq/__init__.pyi
Outdated
| needs: dict[str, object] | ||
| args: dict[str, object] | ||
|
|
||
| empty: Any |
There was a problem hiding this comment.
The standard pattern for this kind of exported sentinel in typeshed is the following:
_Empty = NewType("_Empty", object) # a class at runtime
empty: Final[_Empty]See below for how to use it.
Alternatively, if empty is not supposed to be used by users, I would consider leaving this out.
stubs/punq/punq/__init__.pyi
Outdated
| def register_concrete_service(self, service: type | str, scope: Scope) -> None: ... | ||
| def build_context(self, key: type | str, existing: _ResolutionContext | None = None) -> _ResolutionContext: ... | ||
| def register( | ||
| self, service: type[_T] | str, factory: Callable[..., _T] = ..., instance: _T = ..., scope: Scope = ..., **kwargs: object |
There was a problem hiding this comment.
See above for using _Empty. Also, typeshed uses Any instead of object to mark kwargs where the type matters, but can't be represented accurately.
| self, service: type[_T] | str, factory: Callable[..., _T] = ..., instance: _T = ..., scope: Scope = ..., **kwargs: object | |
| self, service: type[_T] | str, factory: Callable[..., _T] | _Empty = ..., instance: _T | _Empty = ..., scope: Scope = ..., **kwargs: Any # kwargs are passed to ForwardRef |
stubs/punq/punq/__init__.pyi
Outdated
| def register(self, service: type[_TOpt] | str, *, instance: _TOpt, **kwargs: Any) -> Container: ... | ||
| @overload | ||
| def register( | ||
| self, service: type[_TOpt] | str, factory: Callable[..., _TOpt] = ..., *, scope: Scope = ..., **kwargs: Any |
There was a problem hiding this comment.
| self, service: type[_TOpt] | str, factory: Callable[..., _TOpt] = ..., *, scope: Scope = ..., **kwargs: Any | |
| self, service: type[_TOpt] | str, factory: Callable[..., _TOpt] | _Empty = ..., *, scope: Scope = ..., **kwargs: Any |
stubs/punq/punq/__init__.pyi
Outdated
| def __getitem__(self, key: type | str) -> object: ... | ||
| def __setitem__(self, key: type | str, value: object) -> None: ... |
There was a problem hiding this comment.
| def __getitem__(self, key: type | str) -> object: ... | |
| def __setitem__(self, key: type | str, value: object) -> None: ... | |
| def __getitem__(self, key: type | str) -> Any: ... # return type depends on the cache entry | |
| def __setitem__(self, key: type | str, value: Any) -> None: ... # value type depends on the cache entry |
There was a problem hiding this comment.
Updated to type these generically instead of using Any
| class Container: | ||
| registrations: _Registry | ||
| def __init__(self) -> None: ... | ||
| @overload |
There was a problem hiding this comment.
| @overload | |
| # kwargs are passed to ForwardRef | |
| @overload |
There was a problem hiding this comment.
They're actually all eventually passed to builder, which is the main destination of all the kwargs for all the functions. I put a single comment at the top of the Container indicating this for all the functions in that class.
stubs/punq/punq/__init__.pyi
Outdated
| service: type[_TOpt] | str, | ||
| factory: Callable[..., _TOpt] = ..., |
There was a problem hiding this comment.
| service: type[_TOpt] | str, | |
| factory: Callable[..., _TOpt] = ..., | |
| service: type[_TOpt] | str | _Empty, | |
| factory: Callable[..., _TOpt] | _Empty = ..., |
There was a problem hiding this comment.
Service can't be empty, I assume you meant factory and instance?
stubs/punq/punq/__init__.pyi
Outdated
| def resolve_all(self, service: type[_TOpt] | str, **kwargs: Any) -> list[_TOpt]: ... | ||
| def resolve(self, service_key: type[_TOpt] | str, **kwargs: Any) -> _TOpt: ... | ||
| def instantiate(self, service_key: type[_TOpt] | str, **kwargs: Any) -> _TOpt: ... |
There was a problem hiding this comment.
Maybe something like this for kwargs?
| def resolve_all(self, service: type[_TOpt] | str, **kwargs: Any) -> list[_TOpt]: ... | |
| def resolve(self, service_key: type[_TOpt] | str, **kwargs: Any) -> _TOpt: ... | |
| def instantiate(self, service_key: type[_TOpt] | str, **kwargs: Any) -> _TOpt: ... | |
| # All kwargs are passed to the builder. | |
| def resolve_all(self, service: type[_TOpt] | str, **kwargs: Any) -> list[_TOpt]: ... | |
| def resolve(self, service_key: type[_TOpt] | str, **kwargs: Any) -> _TOpt: ... | |
| def instantiate(self, service_key: type[_TOpt] | str, **kwargs: Any) -> _TOpt: ... |
stubs/punq/punq/__init__.pyi
Outdated
| class _Registration(NamedTuple, Generic[_T]): | ||
| service: type[_T] | str | ||
| scope: Scope | ||
| builder: Callable[[], _T] |
There was a problem hiding this comment.
It seems that builder can be called with arbitrary keyword args, so this would be more appropriate:
| builder: Callable[[], _T] | |
| builder: Callable[..., _T] |
This comment has been minimized.
This comment has been minimized.
- Replace object with Any where appropriate - Clarify the use of **kwargs in method signatures - Make _T default to Any and use it in more places instead of Any/object - Include _Empty as a default parameter where applicable
This comment has been minimized.
This comment has been minimized.
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
srittau
left a comment
There was a problem hiding this comment.
Thanks! For future reference, please don't force push in typeshed PRs as that makes reviewing changes harder. We squash when merging anyways.
|
Sure, sorry, and thank you for your help |
punq is an IOC (Inversion of Control) Container for Python 3.8+