docker-py
docker-py copied to clipboard
Add types
Part of #2796.
- [ ]
api
(probably not in this PR) - [ ]
context
(same) - [ ]
credentials
(same) - [ ]
models
(same) - [ ]
transport
(same) - [ ]
types
(same) - [ ]
utils
(same) - [x]
auth.py
- [x]
client.py
- [x]
constants.py
- [x]
errors.py
- [x]
tls.py
- [x]
version.py
- [ ] Add an entry to
MAKEFILE
, + items described here
I have used Dict[str, Any]
in auth.py
and client.py
in some places, we might want to narrow the types using TypedDict
instead (or other solutions, can be discussed).
Notes:
-
As adding types will add a lot of imports and changes to the code layout,
isort
and a code formatter could be enforced. -
I suggest we change
json
andbinary
to keyword only arguments. If used as an argument, this could lead to confusion (see here for example). https://github.com/docker/docker-py/blob/82cf559b5a641f53e9035b44b91f829f3b4cca80/docker/api/client.py#L272 -
config_path
andconfig_dict
can be madeOptional
, as this classmethod is already called with optional arguments here. https://github.com/docker/docker-py/blob/82cf559b5a641f53e9035b44b91f829f3b4cca80/docker/auth.py#L153-L164 -
Before continuing, I'd like to know if:
- Using
typing_extensions
is fine (required forLiteral
, introduced in Python3.8) - You want to have type stubs defined either in code, in pyi files, or implemented in typeshed. Some methods require overloads, and this would add a lot of boilerplate in the source code itself, wich is may not be what people contributing to this repository want to see. If we chose to implement stubs in separate pyi files, we could type the public API only, but developers of this library won't benefit from types for internal objects.
- Using
(@aiordache @ulyssessouza @milas; tagging as this is a draft PR).
What I don't see in this MR is:
- something like instructions for how to check what you added with either mypy or pyright
- a test target for a makefile
- info in a Readme
- or literally anything that can help the next person avoid regressions or help them continue your work, respectively.
Could you check to see what conventions (e.g. auto tests, makefile, similar) are used by the project and try to throw together the simplest integration you could think of? Ask for help if you can't figure it out yourself, that's fine :)
What I don't see in this MR is: [...] Could you check to see what conventions (e.g. auto tests, makefile, similar) are used by the project and try to throw together the simplest integration you could think of? Ask for help if you can't figure it out yourself, that's fine :)
As I said here, I was (and still am) waiting for approval from the maintainers of the project. I don't want to add types directly into the project and then see this PR declined because maintainers doesn't want to use them. That's why we are thinking about adding them to typeshed
(see discussion), and I will surely do it if we don't have any feedback from maintainers in the next few days