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

graphviz typing error #1357

Open
barakatzir opened this issue Jan 8, 2025 · 4 comments
Open

graphviz typing error #1357

barakatzir opened this issue Jan 8, 2025 · 4 comments
Labels
bug Something isn't working

Comments

@barakatzir
Copy link

barakatzir commented Jan 8, 2025

Information

graphviz_draw has wrong documentation of return type both in type annotation and docstring. It is marked as PIL.Image when in fact it should be PIL.Image.Image.
This confused me for a good while, since the module is both camel cased and has the same name as the correct return type...

"Doctesting" with pyright, pyright --verifytypes rustworkx, reports the wrong documentation in the type annotation (mypy -p rustworkx misses it due to a # type: ignore on that line) .

  • rustworkx version: 0.15.1
  • Python version: 3.13

Steps to reproduce the problem

Use graphviz_draw anywhere and check your script and check with pyright or mypy, the return type is inferred incorrectly.

# script.py
from __future__ import annotations
from typing import reveal_type
import rustworkx

g: rustworkx.PyGraph[int, int] = rustworkx.PyGraph()
img = rustworkx.visualization.graphviz_draw(g)
reveal_type(img)

In runtime python script.py reports correctly Runtime type is 'PngImageFile' (PngImageFile is a subtype if Image.Image).

The two type checkers fail, one has missing inferred type and the other is wrong:

  • If ran with pyright then output is script.py:8:13 - information: Type of "img" is "Unknown".
  • If ran with mypy then output is script.py:8: note: Revealed type is "None".
@barakatzir barakatzir added the bug Something isn't working label Jan 8, 2025
@IvanIsCoding
Copy link
Collaborator

I need to check if this still applies to main with the latest commit after #1291. I believe the answer will be no.

None is a valid return type for that function though. If you specific a file it just writes to it. Maybe we should use @overload to split the file case with the in-memory case

@barakatzir
Copy link
Author

Ah, sorry. I forgot to check main before opening the issue. It does look as though this is solved in main.

An @overload can be really nice here actually. If you're at it, you can also remove the uneeded # type: ignore if you want.

Thanks again for the quick response and a great package :)

@IvanIsCoding
Copy link
Collaborator

I have quite a few typing PRs open. Once Matthew has time to review them, I will work on the @overload and removing the # type: ignore.

I think the ignore was there initially because Pillow didn't have type stubs initially. And then we never updated it

@barakatzir
Copy link
Author

Both mypy and pyright can be configured to emit an error if there is an unusued # type: ignore.
For mypy --warn-unused-ignores or warn_unused_ignores. For pyright reportUnnecessaryTypeIgnoreComment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants