Reduces the number of ignored mypy errors #4495

xadupre · 2022-09-08T09:56:23Z

Description

Reduces the number of mypy errors. It works with the latest version of mypy. It adds the flag --ignore-missing-package to avoid adding type: ignore every time import numpy is used. The PR removes unnecessary # type: ignore every time an external package is imported.

Motivation and Context

tools/style.sh generates many errors not related to the changes. This PR make it easier to read what is related to the user changes.

jcwchen

I believe in the current codebase a lot of code cannot pass mypy 0.971 check. Currently two CIs are still running two different versions of mypy, which is quite confusing:

mypy 0.782 in Linux-CI
mypy 0.971 in Python Lint CI

It seems there is no way to specify mypy version from the latest action-mypy: tsuyoshicho/action-mypy#48. We probably can use an older version of tsuyoshicho/action-mypy, v2.0.1, which is using 0.790. If there are still a lot of existing errors with 0.790, I would suggest either we keep one version of mypy or upgrade mypy to the latest one (0.971). Upgrade mypy to the latest one will need a lot of fixes and perhaps the best we can do for now is only keeping version 0.782. @@justinchuby What's your thoughts on this? Thanks!

justinchuby · 2022-09-08T15:51:28Z

onnx/__init__.py

@@ -77,7 +77,7 @@ def _serialize(proto: Union[bytes, google.protobuf.message.Message]) -> bytes:
                    "Please use save_as_external_data to save tensors separately from the model file."
                ) from e
            raise
-        return result
+        return result  # type: ignore


We should avoid ignores without error nums (too board)

mypy returns Returning Any from function declared to return "bytes" in this case. I assume SerializeToString() is annotated as Any when this function returns bytes. mypy detects the inconsistency. I don't think we can easliy remove it. I don't know if onnx is the source of the wrong annotation or if it is protobuf.

justinchuby · 2022-09-08T15:52:21Z

action-mypy uses the local mypy if found. We will simply need to install mypy at the version of our choice before running the check.

jcwchen

The part for adding ignore-missing-imports makes sense to me. Thank you for updating so many places!

IMO we should be careful about adding more ignore conditions because it's hard to resolve every of them in the future when feasible. I saw a few places were added # type: ignore so I would like to understand more details and whether we can resolve them properly.

workflow_scripts/test_model_zoo.py

onnx/test/test_external_data.py

onnx/__init__.py

onnx/backend/test/runner/__init__.py

jcwchen · 2022-09-12T16:00:39Z

onnx/backend/test/runner/__init__.py

@@ -178,7 +178,7 @@ def tests(self) -> Type[unittest.TestCase]:
            onnx_backend_tests = BackendTest(backend).tests
        """
        tests = self._get_test_case("OnnxBackendTest")
-        for items_map in sorted(self._filtered_test_items.values()):
+        for items_map in sorted(self._filtered_test_items.values()):  # type: ignore


May I understand which kind of error does it ignore? mypy 0.971 produces: error: Value of type variable "SupportsRichComparisonT" of "sorted" cannot be "Dict[str, TestItem]" [type-var]

jcwchen · 2022-09-12T16:01:01Z

onnx/test/shape_inference_test.py

@@ -100,7 +100,7 @@ def _assert_inferred(
        inferred_model = self._inferred(graph, **kwargs)
        inferred_vis = list(inferred_model.graph.value_info)
        vis = list(sorted(vis, key=lambda x: x.name))
-        inferred_vis = list(sorted(inferred_vis, key=lambda x: x.name))
+        inferred_vis = list(sorted(inferred_vis, key=lambda x: x.name))  # type: ignore


May I understand which kind of error does it ignore? mypy 0.971 produces: error: Returning Any from function declared to return "Union[SupportsDunderLT, SupportsDunderGT]" [no-any-return]

I would still suggest adding the error code in square brackets to narrow the ignore for clarity.

From what I understood, you can ignore this error by forcing the type: type: expected type. But mypy checks the expected type exists and for that it must have been imported. But if I do that, it would raise the warning unused import. So I think # type: ignore is not bad here.

onnx/test/symbolic_shape_test.py

tools/style.sh

* remove unnecessary import Signed-off-by: xadupre <xadupre@microsoft.com> * lint Signed-off-by: xadupre <xadupre@microsoft.com> * black Signed-off-by: xadupre <xadupre@microsoft.com> * black Signed-off-by: xadupre <xadupre@microsoft.com> * lint Signed-off-by: xadupre <xadupre@microsoft.com> * restore old import Signed-off-by: xadupre <xadupre@microsoft.com> Signed-off-by: xadupre <xadupre@microsoft.com>

Signed-off-by: sdpython <xavier.dupre@gmail.com> Signed-off-by: xadupre <xadupre@microsoft.com>

* primary ops to function milestone 1 Signed-off-by: Liqun Fu <liqfu@microsoft.com> * float type Signed-off-by: Liqun Fu <liqfu@microsoft.com> * formatting Signed-off-by: Liqun Fu <liqfu@microsoft.com> * pass backend test Signed-off-by: Liqun Fu <liqfu@microsoft.com> * formatting Signed-off-by: Liqun Fu <liqfu@microsoft.com> * commit Signed-off-by: Liqun Fu <liqfu@microsoft.com> * format Signed-off-by: Liqun Fu <liqfu@microsoft.com> * format Signed-off-by: Liqun Fu <liqfu@microsoft.com> * layernorm Signed-off-by: Liqun Fu <liqfu@microsoft.com> * pass runtime check Signed-off-by: Liqun Fu <liqfu@microsoft.com> * format Signed-off-by: Liqun Fu <liqfu@microsoft.com> * pure function Signed-off-by: Liqun Fu <liqfu@microsoft.com> * format Signed-off-by: Liqun Fu <liqfu@microsoft.com> * reviewer's comments, clean up some Signed-off-by: Liqun Fu <liqfu@microsoft.com> * keep op original version Signed-off-by: Liqun Fu <liqfu@microsoft.com> * fix gtest.function_verify_test Signed-off-by: Liqun Fu <liqfu@microsoft.com> * formatting Signed-off-by: Liqun Fu <liqfu@microsoft.com> * update according to reviewer's comment Signed-off-by: Liqun Fu <liqfu@microsoft.com> Signed-off-by: Liqun Fu <liqfu@microsoft.com> Co-authored-by: G. Ramalingam <grama@microsoft.com> Signed-off-by: xadupre <xadupre@microsoft.com>

Signed-off-by: sdpython <xavier.dupre@gmail.com> Signed-off-by: xadupre <xadupre@microsoft.com>

Signed-off-by: xadupre <xadupre@microsoft.com>

Signed-off-by: sdpython <xavier.dupre@gmail.com> Signed-off-by: xadupre <xadupre@microsoft.com>

…#4470) * apply filesystem from C+17 to handle encoding on Windows Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * add comment in CMakeLists Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * precise msg if missing support Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * void normalize_sep for two types Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * remove typo const Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * remove template functions to header Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * apply clang-format Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * use _wstat Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * use define function to refactor code Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * add required C++ version in readme Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * void char* for std::string in test Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * fix clang-format Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * move wchar_t and wstring only for Windows Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * refactor template Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * typo Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * use char tempalte in template Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * add comments Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * add tests for wstring Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * typo Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * nit comments Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * use existing functions from std::filesystem::path Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * honor != Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * constexpr const char Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * checker disallow absolute path in external tensors; remove related tests Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * fix format Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * add more checker tests Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * black Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * improve comments Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> Signed-off-by: xadupre <xadupre@microsoft.com>

Signed-off-by: sdpython <xavier.dupre@gmail.com> Signed-off-by: xadupre <xadupre@microsoft.com>

Signed-off-by: xadupre <xadupre@microsoft.com>

workflow_scripts/test_model_zoo.py

requirements-dev.txt

Signed-off-by: xadupre <xadupre@microsoft.com>

jcwchen

LGTM. Thank you for enabling the latest mypy!

gramalingam · 2022-09-23T15:55:30Z

Thanks for fixing these!

* Remove unnecessary import (onnx#4484) * remove unnecessary import Signed-off-by: xadupre <xadupre@microsoft.com> * lint Signed-off-by: xadupre <xadupre@microsoft.com> * black Signed-off-by: xadupre <xadupre@microsoft.com> * black Signed-off-by: xadupre <xadupre@microsoft.com> * lint Signed-off-by: xadupre <xadupre@microsoft.com> * restore old import Signed-off-by: xadupre <xadupre@microsoft.com> Signed-off-by: xadupre <xadupre@microsoft.com> * fix mypy issues Signed-off-by: sdpython <xavier.dupre@gmail.com> Signed-off-by: xadupre <xadupre@microsoft.com> * fix mypy issues Signed-off-by: sdpython <xavier.dupre@gmail.com> Signed-off-by: xadupre <xadupre@microsoft.com> * update mypy Signed-off-by: sdpython <xavier.dupre@gmail.com> Signed-off-by: xadupre <xadupre@microsoft.com> * primary ops to function milestone 1 (onnx#4458) * primary ops to function milestone 1 Signed-off-by: Liqun Fu <liqfu@microsoft.com> * float type Signed-off-by: Liqun Fu <liqfu@microsoft.com> * formatting Signed-off-by: Liqun Fu <liqfu@microsoft.com> * pass backend test Signed-off-by: Liqun Fu <liqfu@microsoft.com> * formatting Signed-off-by: Liqun Fu <liqfu@microsoft.com> * commit Signed-off-by: Liqun Fu <liqfu@microsoft.com> * format Signed-off-by: Liqun Fu <liqfu@microsoft.com> * format Signed-off-by: Liqun Fu <liqfu@microsoft.com> * layernorm Signed-off-by: Liqun Fu <liqfu@microsoft.com> * pass runtime check Signed-off-by: Liqun Fu <liqfu@microsoft.com> * format Signed-off-by: Liqun Fu <liqfu@microsoft.com> * pure function Signed-off-by: Liqun Fu <liqfu@microsoft.com> * format Signed-off-by: Liqun Fu <liqfu@microsoft.com> * reviewer's comments, clean up some Signed-off-by: Liqun Fu <liqfu@microsoft.com> * keep op original version Signed-off-by: Liqun Fu <liqfu@microsoft.com> * fix gtest.function_verify_test Signed-off-by: Liqun Fu <liqfu@microsoft.com> * formatting Signed-off-by: Liqun Fu <liqfu@microsoft.com> * update according to reviewer's comment Signed-off-by: Liqun Fu <liqfu@microsoft.com> Signed-off-by: Liqun Fu <liqfu@microsoft.com> Co-authored-by: G. Ramalingam <grama@microsoft.com> Signed-off-by: xadupre <xadupre@microsoft.com> * lint Signed-off-by: sdpython <xavier.dupre@gmail.com> Signed-off-by: xadupre <xadupre@microsoft.com> * remove # type: ignore for imports Signed-off-by: xadupre <xadupre@microsoft.com> * lint Signed-off-by: sdpython <xavier.dupre@gmail.com> Signed-off-by: xadupre <xadupre@microsoft.com> * Use filesystem to load filename to prevent encoding issues on Windows (onnx#4470) * apply filesystem from C+17 to handle encoding on Windows Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * add comment in CMakeLists Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * precise msg if missing support Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * void normalize_sep for two types Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * remove typo const Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * remove template functions to header Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * apply clang-format Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * use _wstat Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * use define function to refactor code Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * add required C++ version in readme Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * void char* for std::string in test Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * fix clang-format Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * move wchar_t and wstring only for Windows Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * refactor template Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * typo Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * use char tempalte in template Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * add comments Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * add tests for wstring Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * typo Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * nit comments Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * use existing functions from std::filesystem::path Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * honor != Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * constexpr const char Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * checker disallow absolute path in external tensors; remove related tests Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * fix format Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * add more checker tests Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * black Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * improve comments Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> Signed-off-by: xadupre <xadupre@microsoft.com> * remove unnecessary ignore-missing-type Signed-off-by: sdpython <xavier.dupre@gmail.com> Signed-off-by: xadupre <xadupre@microsoft.com> * remove two type ignore Signed-off-by: xadupre <xadupre@microsoft.com> * lint Signed-off-by: xadupre <xadupre@microsoft.com> * type Signed-off-by: xadupre <xadupre@microsoft.com> Signed-off-by: xadupre <xadupre@microsoft.com> Signed-off-by: sdpython <xavier.dupre@gmail.com> Signed-off-by: Liqun Fu <liqfu@microsoft.com> Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> Co-authored-by: sdpython <xavier.dupre@gmail.com> Co-authored-by: liqun Fu <liqfu@microsoft.com> Co-authored-by: G. Ramalingam <grama@microsoft.com> Co-authored-by: Chun-Wei Chen <jacky82226@gmail.com>

xadupre requested review from a team as code owners September 8, 2022 09:56

jcwchen reviewed Sep 8, 2022

View reviewed changes

justinchuby reviewed Sep 8, 2022

View reviewed changes

jcwchen mentioned this pull request Sep 8, 2022

Install specified mypy version first in Python Lint CI to make mypy version consistent #4498

Merged

xadupre requested review from jcwchen and justinchuby September 12, 2022 14:06

jcwchen reviewed Sep 12, 2022

View reviewed changes

justinchuby reviewed Sep 13, 2022

View reviewed changes

tools/style.sh Outdated Show resolved Hide resolved

xadupre and others added 13 commits September 14, 2022 15:53

fix mypy issues

6e7c7ea

Signed-off-by: sdpython <xavier.dupre@gmail.com> Signed-off-by: xadupre <xadupre@microsoft.com>

fix mypy issues

1b90ac2

Signed-off-by: sdpython <xavier.dupre@gmail.com> Signed-off-by: xadupre <xadupre@microsoft.com>

update mypy

d5a6414

Signed-off-by: sdpython <xavier.dupre@gmail.com> Signed-off-by: xadupre <xadupre@microsoft.com>

lint

230b435

Signed-off-by: sdpython <xavier.dupre@gmail.com> Signed-off-by: xadupre <xadupre@microsoft.com>

remove # type: ignore for imports

642ba23

Signed-off-by: xadupre <xadupre@microsoft.com>

lint

aee6153

Signed-off-by: sdpython <xavier.dupre@gmail.com> Signed-off-by: xadupre <xadupre@microsoft.com>

remove unnecessary ignore-missing-type

9ea3b41

Signed-off-by: sdpython <xavier.dupre@gmail.com> Signed-off-by: xadupre <xadupre@microsoft.com>

remove two type ignore

f04431f

Signed-off-by: xadupre <xadupre@microsoft.com>

lint

9ae1994

Signed-off-by: xadupre <xadupre@microsoft.com>

Merge branch 'main' into style2

aa6ca40

xadupre requested a review from jcwchen September 14, 2022 17:26

jcwchen reviewed Sep 14, 2022

View reviewed changes

workflow_scripts/test_model_zoo.py Outdated Show resolved Hide resolved

requirements-dev.txt Outdated Show resolved Hide resolved

type

3290a85

Signed-off-by: xadupre <xadupre@microsoft.com>

jcwchen approved these changes Sep 14, 2022

View reviewed changes

xadupre added 3 commits September 15, 2022 09:57

Merge branch 'main' of https://github.com/onnx/onnx into style2

d0c6e14

Merge branch 'main' of https://github.com/onnx/onnx into style2

6605752

Merge branch 'main' of https://github.com/onnx/onnx into style2

6d2eef3

Merge branch 'main' into style2

4320e58

gramalingam approved these changes Sep 23, 2022

View reviewed changes

gramalingam enabled auto-merge (squash) September 23, 2022 15:55

gramalingam merged commit 7f0a9f2 into onnx:main Sep 23, 2022

jcwchen mentioned this pull request Sep 23, 2022

Upgrade mypy to latest version #3964

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reduces the number of ignored mypy errors #4495

Reduces the number of ignored mypy errors #4495

xadupre commented Sep 8, 2022 •

edited

jcwchen left a comment

justinchuby Sep 8, 2022

xadupre Sep 13, 2022

justinchuby commented Sep 8, 2022

jcwchen left a comment

jcwchen Sep 12, 2022 •

edited

jcwchen Sep 12, 2022 •

edited

justinchuby Sep 13, 2022

xadupre Sep 13, 2022

jcwchen left a comment

gramalingam commented Sep 23, 2022

Reduces the number of ignored mypy errors #4495

Reduces the number of ignored mypy errors #4495

Conversation

xadupre commented Sep 8, 2022 • edited

Description

Motivation and Context

jcwchen left a comment

Choose a reason for hiding this comment

justinchuby Sep 8, 2022

Choose a reason for hiding this comment

xadupre Sep 13, 2022

Choose a reason for hiding this comment

justinchuby commented Sep 8, 2022

jcwchen left a comment

Choose a reason for hiding this comment

jcwchen Sep 12, 2022 • edited

Choose a reason for hiding this comment

jcwchen Sep 12, 2022 • edited

Choose a reason for hiding this comment

justinchuby Sep 13, 2022

Choose a reason for hiding this comment

xadupre Sep 13, 2022

Choose a reason for hiding this comment

jcwchen left a comment

Choose a reason for hiding this comment

gramalingam commented Sep 23, 2022

xadupre commented Sep 8, 2022 •

edited

jcwchen Sep 12, 2022 •

edited

jcwchen Sep 12, 2022 •

edited