Closed kevinsung closed 2 years ago
Probably a Python 3.9 issue.
I just started seeing this and was wondering whether it's only my machine. I'm on python 3.8 but I've seen it on python 3.7. I wonder why the CI is not catching it.
This is because of the release of numpy 1.20, which added type annotations
This isn't picked up by the CI since we don't install cirq or its requirements during the mypy check
diff --git a/dev_tools/conf/mypy.ini b/dev_tools/conf/mypy.ini
index f931717b..6fbc7276 100644
--- a/dev_tools/conf/mypy.ini
+++ b/dev_tools/conf/mypy.ini
@@ -5,10 +5,15 @@ follow_imports = silent
ignore_missing_imports = true
# 3rd-party libs for which we don't have stubs
-[mypy-apiclient.*,freezegun.*,matplotlib.*,mpl_toolkits,multiprocessing.dummy,numpy.*,oauth2client.*,pandas.*,pytest.*,scipy.*,sortedcontainers.*,setuptools.*,pylatex.*,networkx.*,qiskit.*,pypandoc.*,ply.*,_pytest.*,google.api.*,google.api_core.*,grpc.*,google.oauth2.*,google.protobuf.text_format.*,quimb.*,pyquil.*,google.cloud.*,filelock.*,codeowners.*,tqdm.*]
+[mypy-apiclient.*,freezegun.*,matplotlib.*,mpl_toolkits,multiprocessing.dummy,oauth2client.*,pandas.*,pytest.*,scipy.*,sortedcontainers.*,setuptools.*,pylatex.*,networkx.*,qiskit.*,pypandoc.*,ply.*,_pytest.*,google.api.*,google.api_core.*,grpc.*,google.oauth2.*,google.protobuf.text_format.*,quimb.*,pyquil.*,google.cloud.*,filelock.*,codeowners.*,tqdm.*]
follow_imports = silent
ignore_missing_imports = true
+# There was no type information before numpy 1.20
+[mypy-numpy.*]
+follow_imports = skip
+follow_imports_for_stubs = true
+
#Adding "sympy.* or mypy-sympy to the above list (3rd-party libs for which we don't have stubs) doesn't ignore "cannot find module 'sympy' error
[mypy-sympy.*]
ignore_missing_imports = True
silences it
Thanks for the research on this and silencing tip @mpharrigan! I believe that we should slowly chip away at this though - there is a reason why we have mypy - thus if we believe in the usefulness of type checking, we are behooved to follow through with fixing these issues and then turn it on on CI (by installing the correct requirements).
What's the plan for this? I'm getting agita from having my uncomitted mypy.ini
change hanging out on all my branches so I can actually run mypy locally without being drowned by numpy errors.
FWIW we've internally used the follow_imports_for_stubs = true
config that you mentioned above to allow us to update to numpy 1.20 and then incrementally fix type annotations. We have a "mypy-next" CI job that runs mypy with a different configuration where we do follow stub imports; errors in that job do not fail the build but we can look at them in the log so we can see what's left to do. We've also used this mypy-next scheme to incrementally fix the code base as we change various mypy flags.
+1 to that approach - @mpharrigan do you want to implement it?
Do you have a failing check and have github ignore the result or do you put || true
in the check, i.e. always have it report success and rely on manually inspecting the log to see what's really going on?
Changing to kind/health instead of kind/bug-report since nothing is currently broken
Run check/mypy --next Success: no issues found in 966 source files
Looks like @mpharrigan squashed all the bugs. Closing.
I'm pretty sure there are a ton of (num+my)py bugs left. check/mypy --next
assumes you have numpy >= 1.20 or it won't do the new numpy checks.
In any event, if it is fixed, we'd need to modify the CI to only do the check/mypy --next
behavior (and remove the flag, probably)
Having said that, I see that check/mypy --next
doesn't find anything on my machine.
What is the goal for before-1.0 here? I made progress on fixing the numpy errors in check/mypy --next
in https://github.com/quantumlib/Cirq/pull/5227, but if we want to fix all the remaining ~100 errors it's going to be quite a bit of work. This seems like something that could reasonably be postponed until after 1.0, though I do think it'd be good to continue to push on this so we can enable mypy checking of numpy calls eventually.
Discussion from cirq cync: this is a nice to have for cirq 1.0, since typing changes could theoretically change the API interface, but these would likely be small changes and may not materialize. We will keep it on before 1.0 for now, but if we run out of time, this is a good candidate to cut.
Description of the issue mypy encounters errors.
How to reproduce the issue In a fresh virtual environment, install Cirq and the dev tools (mypy version 0.782). Then execute
This gives the following errors:
Cirq version f08b3444004e44b6c0cebf0ad4136775f8decdf9