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

Add py.typed files to graphviz packages #180

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ritikmishra
Copy link

The graphviz library uses type annotations widely, which is very useful. However, currently when using master, attempting to use mypy on a file that imports graphviz gives the following error:

$ echo "import graphviz" > foo.py
$ mypy foo.py
foo.py:1: error: Skipping analyzing "graphviz": module is installed, but missing library stubs or py.typed marker
foo.py:1: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
Found 1 error in 1 file (checked 1 source file)

To make the graphviz type annotations accessible to dependent projects, I added py.typed files to graphviz, graphviz.backend, and graphviz.parameters as described in the MyPy documentation.

@eliotwrobson
Copy link

Bump on this? This would be very useful.

@xflr6
Copy link
Owner

xflr6 commented Jul 22, 2023

Thanks for the PR and sorry for the late response.

Let me explain: a while ago during refactoring the main graphviz classes into smaller classes internally using multiple inheritance and cooperative super()-calls, the https://google.github.io/pytype/ test started failing with an internal error message that hinted at a pytype bug to be reported (currently this does not fail the whole build but is rather treated as a warning):

- name: Run pytype (if -pytype)
if: ${{ contains(matrix.name, '-pytype') }}
run: |
echo "::group::Run pip install pytype"
pip install pytype
echo "::endgroup::"
echo "::group::Run pytype"
FAILED=0
pytype || FAILED=$?
echo "::endgroup::"
[ $FAILED -eq 0 ] || echo "::warning::pytype failed with exit code $FAILED"
shell: bash

Because pytype stops at the first finding, this meant that we could not check the type annotations any more. I would in general prefer for them to be checked before marking https://peps.python.org/pep-0561/.

Recently, the pytype error message changed into a 'normal' error that still looks to me very much like a false alarm. I think I just managed to silence the remaining false positives in these commits (actually, the second one was an actual imprecise annotation):

With pytype now passing again after a long time, I think we can promote failed pytype from warning to build-blocking error again and also mark py.typed but maybe it would be good before to take another look at the probable false positive (this might be related to the mro) and the type annotations to make sure they are correct.

@gtback
Copy link

gtback commented Oct 10, 2023

I would be interested in helping out with this. I confirm that pytype is returning zero errors in a local venv with Python 3.10:

❯ pytype
Computing dependencies
Analyzing 33 sources with 0 local dependencies
ninja: Entering directory `.pytype'
[43/43] check graphviz.unflattening
Leaving directory '.pytype'
Success: no errors found

I tried mypy (which I'm using in my project and would like to have understand calls to graphviz). It raised a few errors but nothing that I don't think could be cleaned up.

@cbornet
Copy link

cbornet commented Jul 30, 2024

This would be super useful. Can it get some attention again ?

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (6199703) to head (272ba9f).
Report is 66 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##            master      #180    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           33        28     -5     
  Lines         1089       952   -137     
==========================================
- Hits          1089       952   -137     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

Successfully merging this pull request may close these issues.

6 participants