Add Mypy for static type checking #2808

seisman · 2023-11-13T09:46:16Z

Description of proposed changes

References:

https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports

Part of #2794

Reminders

Run make format and make check to make sure the code follows the style guide.
Add tests for new features or tests that would have caught the bug that you're fixing.
Add new public functions/methods/classes to doc/api/index.rst.
Write detailed docstrings for all functions/methods.
If wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
If adding new functionality, add an example to docstrings or tutorials.
Use underscores (not hyphens) in names of Python files and directories.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash commands are:

/format: automatically format and lint the code
/test-gmt-dev: run full tests on the latest GMT development version

seisman · 2023-11-16T08:16:12Z

pygmt/figure.py

@@ -518,7 +518,7 @@ def _repr_html_(self):
        html = '<img src="data:image/png;base64,{image}" width="{width}px">'
        return html.format(image=base64_png.decode("utf-8"), width=500)

-    from pygmt.src import (  # pylint: disable=import-outside-toplevel
+    from pygmt.src import (  # type: ignore # pylint: disable=import-outside-toplevel


To suppress the following error:

pygmt/figure.py:521: error: Unsupported class scoped import [misc]

seisman · 2023-11-16T08:18:22Z

pyproject.toml

@@ -86,6 +86,10 @@ make-summary-multi-line = true
 wrap-summaries = 79
 wrap-descriptions = 79

+[tool.mypy]
+exclude = ["pygmt/tests/"]
+ignore_missing_imports = true


Some packages like geopandas don't provide type hints and mypy reports following errors:

Cannot find implementation or library stub for module named "geopandas" [import-not-found]

Ignore these errors following https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports.

seisman · 2023-11-16T08:19:10Z

pygmt/datasets/load_remote_dataset.py

@@ -58,7 +58,7 @@ class GMTRemoteDataset(NamedTuple):
    title: str
    name: str
    long_name: str
-    units: str
+    units: Union[str, None]


Fix the following error:

pygmt/datasets/load_remote_dataset.py:150: error: Argument "units" to "GMTRemoteDataset" has incompatible type "None"; expected "str" [arg-type]

seisman · 2023-11-26T03:06:09Z

Ping @GenericMappingTools/pygmt-maintainers for review

weiji14

There are a couple of errors on the type check CI at https://github.com/GenericMappingTools/pygmt/actions/runs/6987432529/job/19013968423?pr=2808#step:5:12:

 mypy pygmt
pygmt/src/tilemap.py:11: error: Incompatible types in assignment (expression has type "None", variable has type Module)  [assignment]
pygmt/figure.py:12: error: Incompatible types in assignment (expression has type "None", variable has type Module)  [assignment]
Found 2 errors in 2 files (checked 87 source files)
make: *** [Makefile:78: typecheck] Error 1

Can those be fixed?

seisman · 2023-11-27T04:28:18Z

There are a couple of errors on the type check CI at https://github.com/GenericMappingTools/pygmt/actions/runs/6987432529/job/19013968423?pr=2808#step:5:12:
 mypy pygmt
pygmt/src/tilemap.py:11: error: Incompatible types in assignment (expression has type "None", variable has type Module)  [assignment]
pygmt/figure.py:12: error: Incompatible types in assignment (expression has type "None", variable has type Module)  [assignment]
Found 2 errors in 2 files (checked 87 source files)
make: *** [Makefile:78: typecheck] Error 1
Can those be fixed?

They will be fixed in #2809

.github/workflows/typecheck.yml

Makefile

weiji14 · 2023-11-27T04:37:51Z

.github/workflows/typecheck.yml

+          # Need to install four groups of packages:
+          # 1. required packages
+          # 2. optional packages
+          # 3. type checker and stub package
+          # 4. other packages that are used somewhere in PyGMT
+          python -m pip install \
+            numpy pandas xarray netcdf4 packaging \
+            contextily geopandas ipython rioxarray \
+            mypy pandas-stubs \
+            matplotlib pytest
+          python -m pip list


Haven't used this before, but could we use mypy --install-types ... following https://github.com/pydata/xarray/blob/f7edd1e8c2d6d1a8668cc8baba9e1c66d87b8bb4/.github/workflows/ci-additional.yaml#L124C14-L124C14 instead of listing all these dependencies?

It doesn't work. The doc says:

This flag causes mypy to install known missing stub packages for third-party libraries using pip.

So it only works for pandas which maintains a separate stub package pandas-stubs and can't install packages like numpy.

I tried to create a fresh environment and run

$ conda create --name test python=3.12 mypy $ conda activate test $ mypy pygmt pygmt/io.py:4: error: Cannot find implementation or library stub for module named "xarray" [import-not-found] pygmt/datasets/tile_map.py:7: error: Cannot find implementation or library stub for module named "contextily" [import-not-found] pygmt/datasets/tile_map.py:11: error: Cannot find implementation or library stub for module named "numpy" [import-not-found] ... # A lot of similar errors Installing missing stub packages: python -m pip install pandas-stubs Install? [yN]

Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>

weiji14 · 2023-12-01T23:03:23Z

Looking at astral-sh/ruff#3893, there's a Rust-based type checker at https://github.com/mtshiba/pylyzer. Should we consider that too?

seisman · 2023-12-02T03:32:54Z

Looking at astral-sh/ruff#3893, there's a Rust-based type checker at https://github.com/mtshiba/pylyzer. Should we consider that too?

Mypy is the de facto static type checker for Python, so it makes sense to start type hints with Mypy. We may consider switching to other tools like pyright or pylyzer later.

weiji14

Just one tiny suggestion, otherwise merge #2809 first before this one.

weiji14 · 2023-12-04T04:54:43Z

environment.yml

+    # Dev dependencies (type hints)
+    - mypy
+    - pandas-stubs


Maybe move this under the # Dev dependencies (style checks) section?

I prefer to keep them in a separate section.

seisman added 6 commits November 13, 2023 17:26

Add mypy as a dependency

c1bd0be

Add a make target for static type check

594e07e

Run the static type check in the Style Checks workflow

16da12b

Ignore all tests

fec7e8b

Add pandas-stub

ebe7d50

Install more packages

85e7128

seisman force-pushed the typehints/mypy branch from 6165a1a to 85e7128 Compare November 14, 2023 04:22

seisman added 2 commits November 14, 2023 14:40

GMTRemoteDataset's units can be None

2310e7e

Remove modules that don't support type hints

9dc8c98

seisman force-pushed the typehints/mypy branch from e0698d4 to 2355292 Compare November 14, 2023 12:42

Set ignore_missing_imports to true

dda26d4

seisman force-pushed the typehints/mypy branch from 2355292 to dda26d4 Compare November 14, 2023 12:45

seisman mentioned this pull request Nov 15, 2023

Support Type Hints #2794

Open

3 tasks

seisman added the typing Type hints and static type checking label Nov 15, 2023

seisman added 4 commits November 15, 2023 22:48

Ignore the 'Unsupported class scoped import [misc]' error

97d3dbc

Add a new workflow for static type checking

8b2a5b9

Merge branch 'main' into typehints/mypy

aed77ad

Add comments to typecheck workflow

9fd5a04

seisman commented Nov 16, 2023

View reviewed changes

seisman marked this pull request as ready for review November 16, 2023 08:19

seisman added this to the 0.11.0 milestone Nov 16, 2023

seisman added the maintenance Boring but important stuff for the core devs label Nov 16, 2023

seisman changed the title ~~Use Mypy for static type checking~~ Add Mypy for static type checking Nov 16, 2023

Merge branch 'main' into typehints/mypy

fa44524

seisman added the needs review This PR has higher priority and needs review. label Nov 18, 2023

seisman added 3 commits November 18, 2023 23:13

Ignore .mypy_cache and run 'make clean' to delete it

e62dcb6

Merge branch 'main' into typehints/mypy

ec25885

Merge branch 'main' into typehints/mypy

73c9863

seisman requested a review from a team November 22, 2023 10:44

Merge branch 'main' into typehints/mypy

4d31ce3

weiji14 reviewed Nov 27, 2023

View reviewed changes

seisman and others added 7 commits November 27, 2023 13:14

Apply suggestions from code review

a7545ad

Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>

Rename typecheck.yml -> type_checks.ym

49e67d5

Merge branch 'main' into typehints/mypy

253a491

Fix some typos

567bfb8

Merge branch 'main' into typehints/mypy

5e401c2

Merge branch 'main' into typehints/mypy

cd9d3ed

Merge branch 'main' into typehints/mypy

9bfe49a

seisman requested a review from weiji14 December 1, 2023 06:57

Ignore a specific error code

f230b0d

Merge branch 'main' into typehints/mypy

0e657bd

weiji14 approved these changes Dec 4, 2023

View reviewed changes

weiji14 added final review call This PR requires final review and approval from a second reviewer and removed needs review This PR has higher priority and needs review. labels Dec 4, 2023

Merge branch 'main' into typehints/mypy

30b45c1

seisman merged commit 716f622 into main Dec 4, 2023
17 checks passed

seisman deleted the typehints/mypy branch December 4, 2023 09:11

seisman removed the final review call This PR requires final review and approval from a second reviewer label Dec 4, 2023

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Mypy for static type checking #2808

Add Mypy for static type checking #2808

seisman commented Nov 13, 2023 •

edited by weiji14

Loading

seisman Nov 16, 2023

seisman Nov 16, 2023

seisman Nov 16, 2023

seisman commented Nov 26, 2023

weiji14 left a comment

seisman commented Nov 27, 2023

weiji14 Nov 27, 2023

seisman Nov 27, 2023

weiji14 commented Dec 1, 2023

seisman commented Dec 2, 2023

weiji14 left a comment

weiji14 Dec 4, 2023

seisman Dec 4, 2023

Add Mypy for static type checking #2808

Add Mypy for static type checking #2808

Conversation

seisman commented Nov 13, 2023 • edited by weiji14 Loading

seisman Nov 16, 2023

Choose a reason for hiding this comment

seisman Nov 16, 2023

Choose a reason for hiding this comment

seisman Nov 16, 2023

Choose a reason for hiding this comment

seisman commented Nov 26, 2023

weiji14 left a comment

Choose a reason for hiding this comment

seisman commented Nov 27, 2023

weiji14 Nov 27, 2023

Choose a reason for hiding this comment

seisman Nov 27, 2023

Choose a reason for hiding this comment

weiji14 commented Dec 1, 2023

seisman commented Dec 2, 2023

weiji14 left a comment

Choose a reason for hiding this comment

weiji14 Dec 4, 2023

Choose a reason for hiding this comment

seisman Dec 4, 2023

Choose a reason for hiding this comment

seisman commented Nov 13, 2023 •

edited by weiji14

Loading