quantumlib / Cirq

A Python framework for creating, editing, and invoking Noisy Intermediate Scale Quantum (NISQ) circuits.
Apache License 2.0
4.28k stars 1.02k forks source link

Remove unused "type: ignore" comments to appease mypy #6737

Closed mhucka closed 1 month ago

mhucka commented 1 month ago

Mypy produced errors due to a number of places containing the comment

# type: ignore

Removing those comments made mypy happy.

mhucka commented 1 month ago

I see the CI check failures. This is baffling, because mypy does not produce errors for me locally after I remove the comments – in fact it produced errors when I left them in.

Need to investigate what the difference is between what's happening in the GH workflow and local execution …

pavoljuhas commented 1 month ago

You might have a different mypy version locally then on CI. Please try after

pip install -r dev_tools/requirements/mypy.env.txt

For a truly reproducible local run you may want to use a fresh Python 3.10 environment and redo the CI Type check steps - https://github.com/quantumlib/Cirq/blob/484df6f351e58e02cddfe4b3723c1565c7e6d1d9/.github/workflows/ci.yml#L62-L75

mhucka commented 1 month ago

Well, I use pyenv to create a virtual environment, and then always use

pip install -r dev_tools/requirements/dev.env.txt

to install the Cirq requirements, which in turn causes dev_tools/requirements/mypy.env.txt to be loaded. So, that should hopefully be equivalent. The local mypy I got is version 1.11.1. I can't tell what version we're getting on GH, and while it's true that the latest version is actually 1.11.2, it would seem unlikely that 1.11.1 → 1.11.2 changed this behavior. Unfortunately, the mypy change log does not detail what changed in 1.11.1 or 1.11.2.

But okay, there is clearly a difference somewhere, so I have to chase it down.

pavoljuhas commented 1 month ago

CI uses mypy-1.11.1 as specified in dev_tools/requirements/deps/mypy.txt. Sometimes it helps to remove .pytest_cache/ from local repo in case it has data from a different mypy version. If you still see mypy errors that don't show on the CI, I'd try again with a fresh repo clone and new virtual environment.

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 99.18033% with 1 line in your changes missing coverage. Please review.

Project coverage is 97.83%. Comparing base (484df6f) to head (6605dae). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
conftest.py 75.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #6737 +/- ## ========================================== - Coverage 97.83% 97.83% -0.01% ========================================== Files 1077 1077 Lines 92524 92554 +30 ========================================== + Hits 90523 90552 +29 - Misses 2001 2002 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mhucka commented 1 month ago

Good ideas, and I'll do those tests – thank you. In the interest of time, though, I will close this PR because it's clearly not a problem for the CI workflows, and is most likely some difference in my environment.