Static type checking

[einarwar]

Hey

I think a great addition to this project would be to add a static type checker. I think the two most widely used are pyright and mypy.

Both have support for having some "basic" typechecking initially, then expanding one rule at the time, which is the most feasible way when adding it in a existing project.

I would be happy to provide a PR :) Personally i have most experience with pyright, but open for whatever you think is best

[gabrieldemarmiesse]

Thanks for the suggestion. I love type checking, but I'm really cautious about introducing a static analysis tool in python because of the amount of false positive. Python being a dynamically typing language, I'd prefer the tool to fit our codebase (with minor adjustments) instead of fitting our codebase to the tool. I don't want to restrict contributors by annoying them with false positives or forcing them to add types where it's obvious.

If you're willing to work on this, the path forward would be to add static type checking but with a minimal amount of rules and adding more and more rules progressively and see if our code is compatible. I don't want to tool to force us to write things a certain way, I would like the tool to point actual errors :)

[einarwar]

Type checkers are relatively "new" to Python, and is constantly developed so false positives will absolutely be present.

Speaking from personal experience, I introduced static typing into an already existing codebase. It would be impossible to set "strict" mode at once. I used pyright, and initially set it to Basic. Then I gradually introduced more rules.

As I mentioned earlier, i got most experience with pyright. My main reason for preferring pyright is that it is very actively maintained (several releases per week), and that it is backed by a large company (Microsoft). The main contributor also contributed to several new features in the python language. As can be seen in the repo, the backlog of issues is very small. It also integrates very well with a popular IDE (VSCode). A discussion on the differences between pyright and mypy (might be biased) can be found on the pyright webpage

[gabrieldemarmiesse]

I don't have an opinion about pyright vs mypy. If you want to push on this work and are familiar with pyright, go for it!

[einarwar]

python-on-whales/python_on_whales/command_line_entrypoint.py

Lines 34 to 45 in 338feb5

	@image_app.command()
	def copy_to(
	docker_image: str,
	source: str,
	destination: str,
	new_tag: Optional[str] = None,
	push: bool = False,
	pull: str = "missing",
	):
	docker.image.copy_to(docker_image, source, destination, new_tag, pull)
	if push:
	docker.image.push(new_tag)

Tried to apply the basic set of rules. A typical problem i see is like the example above. The new_tag argument can be str or None, and defaults to None. However, docker.image.push() has the signature

python-on-whales/python_on_whales/components/image/cli_wrapper.py

Line 528 in 338feb5

def push(self, x: Union[str, Iterable[str]], quiet: bool = False) -> None:

That means that if push = True and new_tag = None , docker.image.push() will be called with the unexpected parameter None, and we could run into some unintended behaviour.

What would be the best way to fix this? To keep backwards compatibility, the arguments and its types should probably not be changed. How about raising an exception inside the if push: clause if new_tag == None?

[gabrieldemarmiesse]

Indeed I don't believe this is a false positive here. This is a real error in our code. We are just missing checks. A user could call our function with push=True, new_tag=None and the error message would be pretty ugly (the error message would depend on which lib breaks with the None type). We should have our own user message here explaining to the user why this is not correct and how to fix it. So I agree with you that the solution is to do a type check in the if push.