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

Enable types in podman package #293

Open
ghost opened this issue Jun 18, 2023 · 4 comments
Open

Enable types in podman package #293

ghost opened this issue Jun 18, 2023 · 4 comments
Assignees
Labels
help wanted Extra attention is needed

Comments

@ghost
Copy link

ghost commented Jun 18, 2023

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?

@rhatdan
Copy link
Member

rhatdan commented Jun 20, 2023

SGTM
@jwhonce @umohnani8 WDYT?

@jwhonce
Copy link
Member

jwhonce commented Jun 21, 2023

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.

@ghost
Copy link
Author

ghost commented Jun 25, 2023

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!

@umohnani8 umohnani8 added the help wanted Extra attention is needed label Jan 23, 2024
@jnsnow
Copy link

jnsnow commented Feb 6, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants