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

Update pod component to use generic Iterable and Mapping types #599

Conversation

LewisGaul
Copy link
Collaborator

@LewisGaul LewisGaul commented Jun 1, 2024

Follows #595, working on #584.

@LewisGaul
Copy link
Collaborator Author

LewisGaul commented Sep 1, 2024

I notice I actually used Sequence in some of the other methods... I'm now questioning whether Iterable or Sequence (or something else?) is preferably here, especially considering a string itself is a valid Iterable[str]...

The "strings are iterables of strings" issue is tracked at python/typing#256. The pondering about Iterable vs. Sequence isn't affected though, since strings are also sequences of strings. I think Iterable should be preferable, to allow the user to pass generators (e.g. docker.volume.inspect((x for x in volume_names if x.startswith("foo"))).

Full list of abstract types: https://docs.python.org/3/library/collections.abc.html#collections-abstract-base-classes

Copy link
Owner

@gabrieldemarmiesse gabrieldemarmiesse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks for the patch!

@gabrieldemarmiesse gabrieldemarmiesse merged commit 7bd00cb into gabrieldemarmiesse:master Sep 5, 2024
21 checks passed
gabrieldemarmiesse pushed a commit that referenced this pull request Sep 5, 2024
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

Successfully merging this pull request may close these issues.

2 participants