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

Container Run Return Types #634

Open
eclark0426 opened this issue Sep 12, 2024 · 2 comments
Open

Container Run Return Types #634

eclark0426 opened this issue Sep 12, 2024 · 2 comments

Comments

@eclark0426
Copy link
Contributor

eclark0426 commented Sep 12, 2024

ContainerCLI.run() has several possible return types:

  • A Container when detach is True
  • An Iterable[Tuple[str, bytes]] when stream is True (side comment - this return type is missing from the docstring, which only mentions returning a Container or a str)
  • A str when both detach and stream are False

Currently, when I call run() with detach=True, although I know that it will return a Container, mypy complains about it, since nothing is telling it that the return is a Container and not one of the other possibilities.

It'd be nice if the run method were overloaded to type hint the correct return type based on the input parameters. In most cases, that wouldn't be too bad to overload, but since there are over 100 parameters to the function, having multiple overloads sounds like it'd be tedious to read and maintain.

A possible solution might be to leave the 100 kwargs on the overall signature, then use **kwargs on the overloaded signatures for all kwargs except detach and stream, which seemed like it kept mypy happy without requiring hundreds of duplicated lines of code, but it resulted in VSCode no longer providing tab completion options for the other kwargs. Not sure if the same would happen for other editors, but it wouldn't surprise me.

Any thoughts on how/if the return value could be typed which better takes into account the parameters? I'm happy to implement the changes if there's a good way of doing it! However I didn't want to just go ahead and submit a change that either introduces a ton of duplicated code, or breaks IDE tab completion.

@LewisGaul
Copy link
Collaborator

See #580 and the discussion there :)

@eclark0426
Copy link
Contributor Author

Aha! I had looked around a bit to see if there was anything relevant already, but seems I missed that PR.

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

No branches or pull requests

2 participants