docker-py icon indicating copy to clipboard operation
docker-py copied to clipboard

Type annotations

Open Dreamsorcerer opened this issue 3 years ago • 21 comments

Would be nice to have typing annotations to support static typing in projects with mypy.

Dreamsorcerer avatar Apr 02 '21 11:04 Dreamsorcerer

Hi @Dreamsorcerer

That is a great suggestion. I´m going to set up mypy to the test suite and add typing for future changes.

Thanks and have a nice day :)

feliperuhland avatar Apr 04 '21 13:04 feliperuhland

This would be really great. As an example,

pruned_images = client.images.prune(filters={"dangling": True})

pruned_images is opaque to the editor (in this case PyCharm) because the definition of prune() has no type annotations. As an example, what I did was:

# This requires a third party dependency to support Python 3.6-3.7 as TypedDict is 3.8+.
from typing_extensions import TypedDict

import docker
from launcher.log import log


class PruneResults(TypedDict):
    ImagesDeleted: Optional[int]
    SpaceReclaimed: int

...
pruned_images: PruneResults = client.images.prune(filters={"dangling": True})

If type annotations were used pervasively, users would get this experience out of the box.

The types could then be removed from the docstrings as they wouldn't be needed any more. For example:

https://docker-py.readthedocs.io/en/stable/client.html#docker.client.DockerClient.login

All of the (str) and and (bool) lines in the docs could be moved into type annotations to avoid duplication.

johnthagen avatar Apr 11 '21 21:04 johnthagen

I´m going to set up mypy to the test suite and add typing for future changes.

Let me know if you'd like me to review any PRs, I've done a fair amount of type annotations on other projects.

Dreamsorcerer avatar Apr 11 '21 22:04 Dreamsorcerer

Wanted to throw in my support for this initiative, it'd be grand to have mypy be able to check my code in this fashion. Happy to test out any changes or pair up with someone and help out.

Thanks, +Jonathan

yonkeltron avatar Aug 13 '21 16:08 yonkeltron

Would be great! The situation even got worse with recent versions.

This worked for me with older docker-py versions

import docker
from docker.models.containers import Container
cont: Container = docker.from_env().containers.run("hello-world")

With the most recent version (5.0.3) this throws an

ModuleNotFoundError: No module named 'docker.models'

It feels like docker-py is fighting typing :D

EDIT: Sorry my bad. I wrote bullshit. installed/updated with https://pypi.org/project/docker-py/ instead of https://pypi.org/project/docker/ thats why my code broke

motey avatar Sep 05 '22 14:09 motey

Are there any updates on this issue? I just ran into it recently. Would PR's be welcome to put the type annotations in place as well?

ryanvgates avatar Sep 21 '22 17:09 ryanvgates

@milas Are there any plans to add type-hints to this project?

Eyal-Shalev avatar Dec 05 '22 11:12 Eyal-Shalev

Adding type hints isn't something we've prioritized right now but would certainly review & accept PRs* for!

  • Please use your best judgment re: chunking these up to a reasonable size!

milas avatar Dec 05 '22 20:12 milas

Some packages also just have external type stub packages that can be added separately than the main package -- @Viicos, I see your MR has kind of stalled -- would you be interested in just making a stub package?

Here's an example for redis: https://pypi.org/project/types-redis/

It could start as an external package, then as it gains traction, be directly added to this projec?

rrauenza avatar Jul 05 '23 18:07 rrauenza

Hi @rrauenza, as stated in my PR:

Before continuing, I'd like to know if:

  • Using typing_extensions is fine (required for Literal, 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.

I will open an issue first on typeshed, so that we can make multiple PRs in small chunks that could be tracked there

Viicos avatar Jul 05 '23 19:07 Viicos

It's definitely better to get the annotations inline and within the project, rather than creating a separate stubs library. Typically, the various stub libraries exist because someone else has done them and couldn't get the upstream project to include typing. As more libraries have started to adopt them, those libraries should be gradually disappearing.

Dreamsorcerer avatar Jul 06 '23 16:07 Dreamsorcerer

Annotations can still be done in small PRs, let me know if you need help setting it up.

Dreamsorcerer avatar Jul 06 '23 16:07 Dreamsorcerer

It's definitely better to get the annotations inline and within the project, rather than creating a separate stubs library. Typically, the various stub libraries exist because someone else has done them and couldn't get the upstream project to include typing. As more libraries have started to adopt them, those libraries should be gradually disappearing.

I agree, but in the end it will probably be up to the maintainers. That's why I asked about this (https://github.com/docker/docker-py/issues/2796#issuecomment-1622335584). Unfortunately the maintainers haven't responded yet

Viicos avatar Jul 06 '23 16:07 Viicos

Gotta weigh in here - please if at all possible, strongly prefer accepting type hints into the project over having to create a separately maintained, separately released module with typeshed to be installed with e.g. types-docker.

GhostLyrics avatar Jul 07 '23 11:07 GhostLyrics

Gotta weigh in here - please if at all possible, strongly prefer accepting type hints into the project over having to create a separately maintained, separately released module with typeshed to be installed with e.g. types-docker.

I agree, but this issue has been open for 2 years ...

rrauenza avatar Jul 07 '23 14:07 rrauenza

Is there any hope for typehints in the future?

FeryET avatar Aug 27 '23 09:08 FeryET

Would be great! The situation even got worse with recent versions.

This worked for me with older docker-py versions

import docker
from docker.models.containers import Container
cont: Container = docker.from_env().containers.run("hello-world")

With the most recent version (5.0.3) this throws an

ModuleNotFoundError: No module named 'docker.models'

It feels like docker-py is fighting typing :D

EDIT: Sorry my bad. I wrote bullshit. installed/updated with https://pypi.org/project/docker-py/ instead of https://pypi.org/project/docker/ thats why my code broke

Your example actually gives an error in PyRight:

error: Expression of type "Model | Unknown | bytes | None" cannot be assigned to declared type "Container"
    Type "Model | Unknown | bytes | None" cannot be assigned to type "Container"

I've not looked into it but it's the first time I've encountered anything like this. I'm having to spam noqa's everywhere when using this library, because I'd rather preserve the correct type hint Container than annotate with Model | Unknown | bytes | None which is a bit crazy.

martimors avatar Sep 08 '23 14:09 martimors

I made a stubs repo, feel free to contribute: https://github.com/rdozier-work/docker-stubs

rdozier-work avatar Oct 02 '23 18:10 rdozier-work

@rdozier-work thank you! it would be great to have a PyPI package for docker-stubs. meanwhile

docker-stubs @ git+https://github.com/rdozier-work/docker-stubs@9de7906804ae912f1d644c97b617ac77e784fca8

dalazx avatar Nov 07 '23 13:11 dalazx

I'd be happy to put it on PyPI, I'd just like to have it mature a bit first, since at the moment it's basically just stubgen

rdozier-work avatar Nov 07 '23 15:11 rdozier-work

@rdozier-work if you add this to PyPI, I expect folks (including me) will start using it, and then the contributions will pick up.

adamtheturtle avatar Jan 24 '24 12:01 adamtheturtle