[docker] Add missing type annotations in docker.types#15566
[docker] Add missing type annotations in docker.types#15566emmanuel-ferdman wants to merge 1 commit intopython:mainfrom
Conversation
Signed-off-by: Emmanuel Ferdman <emmanuelferdman@gmail.com>
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
srittau
left a comment
srittau
left a comment
There was a problem hiding this comment.
Some preliminary notes about Any, not a full review.
Please note that each use of Any must be well-argument using code comments.
| class DictType(dict[str, Any]): | ||
| def __init__(self, init: Mapping[str, Any]) -> None: ... |
There was a problem hiding this comment.
I don't think we should use Any as value type. If the dict is generic over the value type, we should use a TypeVar like dict does.
| class LogConfig(DictType): | ||
| types: type[LogConfigTypesEnum] | ||
| def __init__(self, **kwargs) -> None: ... | ||
| def __init__(self, **kwargs: Any) -> None: ... |
There was a problem hiding this comment.
Any is not the best we can do here. Looking at the code, LogConfig.__init__ supports two keyword arguments type and config (which may also be spelled Type and Config). All other keyword arguments are ignored. We should represent that in the stubs. For example, like this:
| def __init__(self, **kwargs: Any) -> None: ... | |
| def __init__(self, *, type: str, Type: str, config: dict[str, Incomplete], Config: dict[str, Incomplete]) -> None: ... |
or this
| def __init__(self, **kwargs: Any) -> None: ... | |
| def __init__(self, *, type: str, Type: str, config: dict[str, Incomplete], Config: dict[str, Incomplete], **kwargs: Unused) -> None: ... |
(Depending on whether providing extra, unused keyword arguments is expected or as potential error.)
An advanced version would use overloads to prevent type and Type or config and Config at the same time.
2 participants
PR Summary
This PR replaces all
Incompletetypes indocker.typeswith specific types verified against the docker-py runtime.