-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Use context managers for disable_errors() #10251
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution!
mypy/messages.py
Outdated
self.disable_count += 1 | ||
yield |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should use try-finally
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the review @JelleZijlstra
yield should be inside try-finally?
@contextmanager
def disable_errors(self) -> Iterator[None]:
self.disable_count += 1
try:
yield
finally:
pass
self.disable_count -= 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And the -= 1
should be in the finally block. The point is to make sure the cleanup code always happens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JelleZijlstra Now that we have try-finally inside enable_errors()
below try-finally is redundent right? Can be removed?
Line 2811 in 8860575
right_type = self.analyze_cond_branch(right_map, e.right, left_type) |
if right_map is None:
with self.msg.disable_errors():
try:
right_type = self.analyze_cond_branch(right_map, e.right, left_type)
finally:
pass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A try-finally with an empty finally block doesn't do anything useful, so yes you can remove it.
self.disable_count += 1 | ||
yield | ||
self.disable_count -= 1 | ||
|
||
def enable_errors(self) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this method be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed enable_errors()
Getting below error. |
Getting below error. Do you ahve any suggestions? |
Sorry, I'm not too familiar with that code. I'd try adding an annotation though, as the error message suggests. |
58c4ded
to
c7a2b5c
Compare
Diff from mypy_primer, showing the effect of this PR on open source code: flake8 (https://github.com/pycqa/flake8.git)
+ src/flake8/checker.py:471: error: "Optional[FileProcessor]" has no attribute "build_ast"
+ src/flake8/checker.py:497: error: "Optional[FileProcessor]" has no attribute "build_logical_line"
+ src/flake8/checker.py:500: error: "Optional[FileProcessor]" has no attribute "update_state"
+ src/flake8/checker.py:505: error: "Optional[FileProcessor]" has no attribute "update_checker_state_for"
+ src/flake8/checker.py:518: error: "Optional[FileProcessor]" has no attribute "next_logical_line"
+ src/flake8/checker.py:526: error: "Optional[FileProcessor]" has no attribute "update_checker_state_for"
+ src/flake8/checker.py:561: error: "Optional[FileProcessor]" has no attribute "generate_tokens"
+ src/flake8/checker.py:599: error: "Optional[FileProcessor]" has no attribute "reset_blank_before"
+ src/flake8/checker.py:602: error: "Optional[FileProcessor]" has no attribute "visited_new_blank_line"
+ src/flake8/checker.py:603: error: "Optional[FileProcessor]" has no attribute "delete_first_token"
+ src/flake8/checker.py:633: error: "Optional[FileProcessor]" has no attribute "inside_multiline"
+ src/flake8/checker.py:634: error: "Optional[FileProcessor]" has no attribute "split_line"
pydantic (https://github.com/samuelcolvin/pydantic.git)
+ pydantic/validators.py:195: error: Unsupported operand types for < ("int" and "None") [operator]
+ pydantic/validators.py:199: error: Unsupported operand types for > ("int" and "None") [operator]
prefect (https://github.com/PrefectHQ/prefect.git)
+ src/prefect/environments/execution/dask/cloud_provider.py:140: error: "None" has no attribute "scheduler"
pandas (https://github.com/pandas-dev/pandas.git)
+ pandas/io/html.py:1092: error: Unsupported operand types for > ("int" and "None") [operator]
+ pandas/io/html.py:1092: error: Unsupported operand types for > ("int" and "Sequence[int]") [operator]
+ pandas/io/html.py:1092: error: Unsupported operand types for > ("int" and "slice") [operator]
+ pandas/io/html.py:1092: note: Left operand is of type "Union[int, Sequence[int], slice, None]"
+ pandas/tseries/holiday.py:451: error: Value of type "None" is not indexable [index]
aioredis (https://github.com/aio-libs/aioredis.git)
+ aioredis/client.py:3774: error: "Optional[Connection]" has no attribute "read_response"
+ aioredis/client.py:3779: error: "Optional[Match[str]]" has no attribute "groups"
tornado (https://github.com/tornadoweb/tornado.git)
+ tornado/web.py:1715: error: "None" has no attribute "done"
poetry (https://github.com/python-poetry/poetry.git)
+ poetry/utils/appdirs.py:27: error: "Union[str, Path]" has no attribute "startswith"
+ poetry/utils/env.py:1236: error: "Optional[Command]" has no attribute "finalize_options"
+ poetry/utils/env.py:1310: error: "Union[str, int]" has no attribute "strip"
+ poetry/utils/env.py:1321: error: "Union[str, int]" has no attribute "strip"
+ poetry/utils/env.py:1367: error: "Union[int, str]" has no attribute "strip"
+ poetry/version/version_selector.py:54: error: "None" has no attribute "version"
+ poetry/publishing/uploader.py:157: error: "Optional[Match[str]]" has no attribute "group"
spark (https://github.com/apache/spark.git)
+ python/pyspark/shuffle.py:602: error: "Optional[Any]" has no attribute "tell"
+ python/pyspark/shuffle.py:606: error: "Optional[Any]" has no attribute "tell"
pytest (https://github.com/pytest-dev/pytest.git)
+ src/_pytest/fixtures.py:1107: error: "Union[Callable[..., FixtureValue], Callable[..., Generator[FixtureValue, None, None]]]" has no attribute "__get__" [attr-defined]
+ src/_pytest/fixtures.py:1121: error: "Union[Callable[..., FixtureValue], Callable[..., Generator[FixtureValue, None, None]]]" has no attribute "__get__" [attr-defined]
isort (https://github.com/pycqa/isort.git)
+ isort/_vendored/toml/decoder.py:888: error: "Optional[Match[str]]" has no attribute "groups"
graphql-core (https://github.com/graphql-python/graphql-core.git)
+ src/graphql/language/block_string.py:52: error: Unsupported operand types for < ("int" and "None")
|
587937c
to
717e7ff
Compare
Sorry for dropping this! We ended up merging #10569 instead. |
Description
This PR fixes one of the issue reported in #1184
Test Plan
Since this change is coding style change, new cases are not added.