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

Enable types in podman package

Open ghost opened this issue 2 years ago • 4 comments

Adding a py.typed file in the root of the podman package would allow type checking for it in projects that use the package.

For example, the following script:

import podman
client = podman.PodmanClient()
ok = client.containers.exists(1234)
print("Exists: " + ok)

Would not cause any mypy errors (issues would be caught at runtime):

$ mypy --ignore-missing-imports script.py 
Success: no issues found in 1 source file

But with the py.typed file, we can catch the issues earlier:

$ touch venv/lib/python3.11/site-packages/podman/py.typed
$ mypy --ignore-missing-imports script.py 
script.py:3: error: Argument 1 to "exists" of "ContainersManager" has incompatible type "int"; expected "str"  [arg-type]
script.py:4: error: Unsupported operand types for + ("str" and "bool")  [operator]
Found 2 errors in 1 file (checked 1 source file)

Maybe package type issues should be fixed in #292 before enabling type information in the package?

ghost avatar Jun 18 '23 15:06 ghost

SGTM @jwhonce @umohnani8 WDYT?

rhatdan avatar Jun 20 '23 18:06 rhatdan

SGTM, the project has used type hints since day one. I would suggest a survey of which tools exist on our CI platforms and integrate with IDE's like code and idea.

@francisbergin Do you see this new tool a compliment or replacement to the current use of pylint?

Addressing #292 would be a great test of the chosen tool.

jwhonce avatar Jun 21 '23 16:06 jwhonce

Hi!

The chosen tool would be a compliment to Pylint which would focus on type checking. The main tools available that I am aware of are Mypy, Pyright, Pyre and Pytype. I believe the basic functionality should be quite similar between these 4 projects.

I started #294 as a POC of changes required to get the type checker to pass. The first run had 331 type errors and the current run with fixes has 179 errors. In this PR, I adapted the type hints to the code but perhaps in some cases the code needs to be adapted to the type hints.

Please let me know what you think of the changes so far and if it is worth continuing the effort.

Note that for this specific issue, the change would just be to add a py.typed file to the published package which would allow users to type check their usage of the podman package.

Thank you!

ghost avatar Jun 25 '23 16:06 ghost

We've been using mypy for qemu.qmp and for the internal qemu packages in the qemu tree. It's OK. In the areas where it leaves something to be desired, it feels mostly like shortcomings with Python's typing system and not necessarily problems with mypy itself. The error messages are a little misleading and terse at times, but it's overall pretty decent. Cross-version compatibility usually seems to be quite good compared to e.g. pylint which sometimes lags behind new Python beta releases.

I've used vscode and pycharm somewhat sparingly, I don't know what they use to power type checking. (I am unfortunately an emacs gremlin most of the time and can't speak towards the quality of IDE integrations available. Occupational hazard of working with other QEMU developers.)

I have not used Pyright, Pyre nor Pytype. I could explore those other tools to see what they might offer that mypy doesn't; I had some notion that mypy was for better or worse the reference implementation. I know I have seen Pyre discussed at the Python typing summit a few years back, and it seems optimized for speed on huge codebases but I don't know if that focus comes with a caveat or not.

jnsnow avatar Feb 06 '24 21:02 jnsnow