-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
TYP: use mypy_primer to surface type checking regressions #28073
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
Conversation
f069616
to
3aa89c6
Compare
Okay, I think it worked: see the artifacts at https://github.com/numpy/numpy/actions/runs/12524542786?pr=28073. I think the comment will only show up once the workflow is merged, since it needs additional permissions. I sharded mypy across 4 jobs, the longest of which takes 2 minutes. I can additionally run pyright, just let me know, but have a slight preference to do that in a second PR so we can make sure the comment workflow is running correctly. |
This reverts commit 3aa89c6.
I see 4 output diffs, two of which are non-empty: pandas (https://github.com/pandas-dev/pandas)
+ pandas/core/nanops.py:1524: error: Module has no attribute "iscomplexobj" [attr-defined] scipy (https://github.com/scipy/scipy)
+ scipy/linalg/_decomp_ldl.py:4: error: Module "numpy" has no attribute "iscomplexobj" [attr-defined]
+ scipy/linalg/_decomp.py:20: error: Module "numpy" has no attribute "iscomplexobj" [attr-defined]
+ scipy/linalg/_decomp.py:20: error: Module "numpy" has no attribute "iscomplex"; maybe "complex64"? [attr-defined]
+ scipy/sparse/linalg/_isolve/tests/test_iterative.py:10: error: Module "numpy" has no attribute "iscomplexobj" [attr-defined]
+ scipy/interpolate/_cubic.py:501: error: Module has no attribute "iscomplexobj" [attr-defined]
+ scipy/signal/_short_time_fft.py:573: error: Module has no attribute "iscomplexobj" [attr-defined]
+ scipy/signal/_short_time_fft.py:826: error: Module has no attribute "iscomplexobj" [attr-defined] And those seem to be caused by 3aa89c6. So yea, this seems to work 👌🏻. Anyway, let's see what other maintainers have to say about this, before merging this. |
Thank you for working on this @hauntsaninja.
Isn't it simpler to run this in a simple job? This would be preferred I think, since we have a lot of jobs that take >10 minutes and 1 job is much easier to deal with (doesn't clutter list of jobs, only 1 rather than 4 failures if something is off, etc.).
One at a time sounds right to me.
Comments are frowned upon, since they are a source of extra noise/notifications. We don't allow it for other tools, like linters or codecov. And repeated comments for new pushes rather updating an existing comment is a level of noise that's even worse. Is there no way to turn that off? Ideally the diff would be easily reachable from a link in the list of jobs. This kind of CI infra is hard to get working really smoothly; the bot comments seem more acceptable for a separate repo specifically to more easily work on type annotations that @jorenham has started thinking about. |
I agree that that would be most ideal. But if that isn't possible, or too difficult to achieve, perhaps we could only run mypy_primer if there are changes made to |
It should already only run on changes to pyi files: https://github.com/numpy/numpy/pull/28073/files#diff-1d3e91c57336ce4ff8a9d5762ddc5ffa66f99f0afb918aab0b02e53db80bf54cR7 I pushed an update so this now will run in just one job instead of four. Yes, it will comment for subsequent pushes. If you push repeatedly, in flight runs will get cancelled without commenting. Previous comments get hidden automatically. I pushed an update so this will only comment if there is a diff. Based on my experience with this workflow in mypy / typeshed / pyright repos, I think this will be relatively high signal to noise. But let me know if you're still opposed to the comment workflow and I can remove it (now, or later after trying it out). |
Thanks, addressed |
Thanks @hauntsaninja . Let's put this in and take it from there. The comment is still present, we will see how that does. |
Mypy primer seems to run, but doesn't place a comment. E.g. in #28108 I see rclip (https://github.com/yurijmikhalevich/rclip)
- rclip/model.py:212: error: Argument "key" to "sorted" has incompatible type "Callable[[tuple[floating[Any], int]], floating[Any]]"; expected "Callable[[tuple[floating[Any], int]], SupportsDunderLT[Any] | SupportsDunderGT[Any]]" [arg-type]
+ rclip/model.py:212: error: Argument "key" to "sorted" has incompatible type "Callable[[tuple[float64, int]], float64]"; expected "Callable[[tuple[float64, int]], SupportsDunderLT[Any] | SupportsDunderGT[Any]]" [arg-type]
- rclip/model.py:212: error: Incompatible return value type (got "floating[Any]", expected "SupportsDunderLT[Any] | SupportsDunderGT[Any]") [return-value]
+ rclip/model.py:212: error: Incompatible return value type (got "float64", expected "SupportsDunderLT[Any] | SupportsDunderGT[Any]") [return-value]
- rclip/model.py:214: error: Incompatible return value type (got "list[tuple[floating[Any], int]]", expected "list[tuple[float, int]]") [return-value]
+ rclip/model.py:214: error: Incompatible return value type (got "list[tuple[float64, int]]", expected "list[tuple[float, int]]") [return-value]
+ rclip/model.py:214: note: "List" is invariant -- see https://mypy.readthedocs.io/en/stable/common_issues.html#variance
+ rclip/model.py:214: note: Consider using "Sequence" instead, which is covariant
+ rclip/model.py:214: note: Perhaps you need a type annotation for "sorted_similarities"? Suggestion: "list[tuple[float, int]]"
dedupe (https://github.com/dedupeio/dedupe)
- dedupe/clustering.py:249: error: Argument 2 to "confidences" has incompatible type "ndarray[tuple[int, ...], dtype[floating[Any]]]"; expected "ndarray[tuple[int, ...], dtype[float64]]" [arg-type]
- dedupe/clustering.py:280: error: Incompatible types in assignment (expression has type "ndarray[tuple[int, ...], dtype[floating[Any]]]", variable has type "ndarray[tuple[int, ...], dtype[float64]]") [assignment]
pandas (https://github.com/pandas-dev/pandas)
- pandas/core/window/ewm.py:129: error: Incompatible return value type (got "ndarray[tuple[int, ...], dtype[floating[Any]]]", expected "ndarray[tuple[int, ...], dtype[float64]]") [return-value]
+ pandas/core/indexers/objects.py:135: error: Incompatible types in assignment (expression has type "ndarray[tuple[int, ...], dtype[Any]]", variable has type "ndarray[tuple[int], dtype[Any]]") [assignment]
+ pandas/core/indexers/objects.py:405: error: Incompatible types in assignment (expression has type "ndarray[tuple[int, ...], dtype[Any]]", variable has type "ndarray[tuple[int], dtype[Any]]") [assignment]
spark (https://github.com/apache/spark)
- python/pyspark/pandas/frame.py:9354: error: Incompatible types in assignment (expression has type "ndarray[tuple[int, ...], dtype[floating[Any]]]", variable has type "ndarray[tuple[int, ...], dtype[float64]]") [assignment] in the CI logs (https://github.com/numpy/numpy/actions/runs/12637108893/job/35210633407), but no comment 🤔 |
Thanks for posting — and apologies, I'd meant to follow up to check the comment workflow once merged, but hadn't yet gotten the chance to. Looks like I introduced the issue while addressing some feedback on this PR, something like #28130 should make the commenting work |
Resolves #28054. This runs mypy against a bunch of open source projects with PR numpy and merge base numpy, and reports the diff. Nothing in here should block CI.
We use a similar workflow in the pyright, mypy, typeshed, basedpyright repositories. See python/mypy#18278 (comment) for a random example comment.