Fixed type errors in `mypy` GitHub Action by lukeconibear · Pull Request #6963 · pydata/xarray

lukeconibear · 2022-08-28T20:59:28Z

Fixed type errors in `mypy` GitHub Action

Closes Type errors in mypy GitHub Action #6962

Running the GitHub Action for mypy locally returns successful:

$ python -m mypy --install-types --non-interactive 
Success: no issues found in 144 source files

headtr1ck · 2022-08-28T21:36:32Z

That is weird, with from __future__ import annotations this should work.

Actually the pyupgrade pre-commit hook should even replace the typing versions by built-in versions.

headtr1ck · 2022-10-13T09:10:38Z

Could you add an additional mypy workflow to test this behavior?
See #6962 (comment)

headtr1ck · 2022-10-13T14:44:30Z

When I run locally mypy --python-version 3.8 i get more errors. Maybe merge current main and let's see what new issues emerge.

headtr1ck · 2022-10-13T15:00:23Z

@dcherian on a different issue: can we prevent that the auto labeler removes manually added labels?

dcherian · 2022-10-13T15:02:13Z

can we prevent that the auto labeler removes manually added labels?

no idea. cc @TomNicholas

headtr1ck · 2022-10-13T17:14:44Z

Seems that pyupgrade does not update tuple -> Tuple inside of Union, which is exactly what caused these problems.

Looks good to me now 👍

headtr1ck · 2022-10-13T17:29:09Z

Seems like dasks typing is also not compatible with python 3.8, haha

headtr1ck · 2022-10-13T18:02:19Z

I cannot reproduce the mypy error locally.
Does this break for someone else?

max-sixty · 2022-10-13T20:03:50Z

I get some errors:

❯ git describe
v0.17.0-798-g3599f873

❯ mypy --python-version 3.8
xarray/core/types.py:72: error: "tuple" is not subscriptable  [misc]
xarray/core/types.py:74: error: "tuple" is not subscriptable  [misc]
xarray/core/types.py:76: error: "tuple" is not subscriptable  [misc]
xarray/core/types.py:78: error: "list" is not subscriptable  [misc]
xarray/core/_reductions.py:18: error: Cannot find implementation or library stub for module named "flox"  [import]
xarray/core/_reductions.py:18: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
xarray/core/merge.py:43: error: "tuple" is not subscriptable  [misc]
xarray/core/merge.py:44: error: "tuple" is not subscriptable  [misc]
xarray/core/merge.py:45: error: "tuple" is not subscriptable  [misc]
xarray/core/groupby.py:661: error: Cannot find implementation or library stub for module named "flox.xarray"  [import]
xarray/backends/api.py:65: error: "dict" is not subscriptable  [misc]
Found 10 errors in 5 files (checked 139 source files)

(not sure why git describe is showing that tag but the commit hash should repro)

headtr1ck · 2022-10-13T20:07:11Z

Hmmm, these errors should be fixed in the latest version of this PR.

The problem CI is running into is that mypy runs into an unrecoverable error parsing dask, this I cannot reproduce locally.

max-sixty · 2022-10-13T20:07:43Z

Hmmm, these errors should be fixed in the latest version of this PR.

Sorry, my test was on main. I can rerun on this PR.

max-sixty · 2022-10-13T21:29:33Z

I can't repro the failure on the current PR v0.17.0-804-g7018b950. I'm using Python 3.9 with the latest dask.

/home/runner/micromamba/envs/xarray-tests/lib/python3.10/site-packages/dask/utils.py:366: error: Parenthesized context managers are only supported in Python 3.9 and greater  [syntax]
Found 1 error in 1 file (errors prevented further checking)

I'm not sure why it's raising an error for an imported module where --follow-imports=silent. Isn't that supposed to suppress errors in imports?

And weirdly, I get errors running mypy in dask latest main, but not this error!

❯ mypy --python-version 3.8 .
dask/system.py:8: error: Unused "type: ignore" comment
dask/widgets/__init__.py:20: error: All conditional function variants must have identical signatures  [misc]
dask/widgets/__init__.py:23: error: All conditional function variants must have identical signatures  [misc]
dask/layers.py:1322: error: Unused "type: ignore" comment
dask/layers.py:1323: error: Unused "type: ignore" comment
dask/array/core.py:5814: error: Argument 1 to "ndindex" has incompatible type "Tuple[int, ...]"; expected "SupportsIndex"  [arg-type]
dask/array/fft.py:196: error: Unused "type: ignore" comment
dask/dataframe/core.py:438: error: Unused "type: ignore" comment
dask/dataframe/core.py:4186: error: Unused "type: ignore" comment
dask/dataframe/core.py:4197: error: Unused "type: ignore" comment
dask/dataframe/core.py:4209: error: Unused "type: ignore" comment
dask/dataframe/core.py:4408: error: Unused "type: ignore" comment
dask/dataframe/core.py:4420: error: Unused "type: ignore" comment
dask/dataframe/core.py:4425: error: Unused "type: ignore" comment
dask/dataframe/groupby.py:1204: error: Unused "type: ignore" comment
dask/dataframe/io/csv.py:9: error: Unused "type: ignore" comment
dask/bytes/tests/test_s3.py:380: error: Unused "type: ignore" comment
dask/bytes/tests/test_local.py:191: error: Unused "type: ignore" comment
dask/bag/tests/test_text.py:33: error: Unused "type: ignore" comment
dask/dataframe/tests/test_shuffle.py:1408: error: Unused "type: ignore" comment
dask/dataframe/tests/test_shuffle.py:1409: error: Unused "type: ignore" comment
dask/dataframe/tests/test_shuffle.py:1412: error: Unused "type: ignore" comment
dask/diagnostics/tests/test_profiler.py:21: error: Unused "type: ignore" comment
dask/tests/test_graph_manipulation.py:106: error: "tuple" is not subscriptable, use "typing.Tuple" instead  [misc]
Found 24 errors in 14 files (checked 243 source files)

headtr1ck · 2022-10-14T06:29:09Z

It seems that mypy encountered an unrecoverable runtime error.
We should probably report that to mypy, but I cannot reproduce it locally, yet alone create a MVCE.

Illviljan · 2022-11-20T20:12:00Z

Dask is on the ignore list:

xarray/pyproject.toml

Lines 31 to 43 in d6671dd

    
           # Most of the numerical computing stack doesn't have type annotations yet. 
        
           [[tool.mypy.overrides]] 
        
           ignore_missing_imports = true 
        
           module = [ 
        
             "affine.*", 
        
             "bottleneck.*", 
        
             "cartopy.*", 
        
             "cdms2.*", 
        
             "cf_units.*", 
        
             "cfgrib.*", 
        
             "cftime.*", 
        
             "cupy.*", 
        
             "dask.*",

This seems to ignore the list and follow-imports doesn't seem to work either:

      - name: Run mypy with python3.8
        # silent all imports, since external repos might not support this
        run: |
          python -m mypy --install-types --non-interactive --python-version 3.8 --follow-imports=silent

A good ol' copy/paste job works as expected though. :) I think we can discuss more elegant solutions in a follow up PR.

headtr1ck · 2022-11-20T21:44:36Z

A good ol' copy/paste job works as expected though. :) I think we can discuss more elegant solutions in a follow up PR.

So now mypy is not crashing anymore? Thats weird, we should open an issue on mypy about this...

headtr1ck · 2022-11-20T21:45:34Z

.github/workflows/ci-additional.yaml

+        run: |
+          python -m mypy --install-types --non-interactive --cobertura-xml-report mypy_report
+
+      - name: Upload mypy coverage to Codecov


Do we need to upload this to codecov? The coverage should be the same as for the newest python.

Not really, I just kept it simple with the copy/pasting as I suspect we can do this workflow in a smarter way; similar way as the pytest CI or the mypy --python-version 3.8-way.

I think we can just focus on getting the xarray bugs fixed in this PR and not get stuck with the CI semantics.

headtr1ck · 2022-11-20T21:47:34Z

.github/workflows/ci-additional.yaml

+          python xarray/util/print_versions.py
+      - name: Install mypy
+        run: |
+          python -m pip install 'mypy<0.990'


Is mypy=0.990 still crashing?

Illviljan · 2022-11-21T05:59:36Z

A good ol' copy/paste job works as expected though. :) I think we can discuss more elegant solutions in a follow up PR.

So now mypy is not crashing anymore? Thats weird, we should open an issue on mypy about this...

This one throws errors still: python -m mypy --install-types --non-interactive --python-version 3.8 --follow-imports=silent
Did it ever crash if we used the normal mypy CI but with changed python version?

headtr1ck · 2022-11-21T08:04:26Z

This one throws errors still: python -m mypy --install-types --non-interactive --python-version 3.8 --follow-imports=silent
Did it ever crash if we used the normal mypy CI but with changed python version?

I don't think we ever used --python-version outside of this PR.

We should report this mypy bug...

Illviljan · 2022-11-21T21:31:48Z

Thanks, @lukeconibear !

fixed mypy type errors

e1733f3

github-actions bot added io topic-backends labels Aug 28, 2022

headtr1ck added topic-typing and removed topic-backends io labels Oct 13, 2022

Merge branch 'main' into 6962-mypy

4e9ea53

github-actions bot added io topic-backends and removed topic-typing labels Oct 13, 2022

add mypy with python3.8 to ci

1dbae6c

github-actions bot added the Automation Github bots, testing workflows, release automation label Oct 13, 2022

fix mypy python 3.8 problems

afdb930

github-actions bot added the topic-typing label Oct 13, 2022

add verbosity flag to mypy action

6674f93

silence import errors in mypy python 3.8

7018b95

headtr1ck mentioned this pull request Nov 20, 2022

Switch to T_DataArray in .coords #7285

Merged

1 task

Illviljan added 6 commits November 20, 2022 19:25

Merge branch 'main' into pr/6963

50ad45d

Test copying the entire workflow.

69692a6

Update ci-additional.yaml

88e6f7c

Last test running 3.8 inside mypy workflow

d845fec

Update ci-additional.yaml

aabbf7f

Use the copy/pasted workflow only

27cdc33

Illviljan added the plan to merge Final call for comments label Nov 20, 2022

headtr1ck approved these changes Nov 20, 2022

View reviewed changes

headtr1ck approved these changes Nov 21, 2022

View reviewed changes

Illviljan merged commit 88a75c9 into pydata:main Nov 21, 2022

lukeconibear deleted the 6962-mypy branch November 22, 2022 00:09

Uh oh!

Conversation

lukeconibear commented Aug 28, 2022