Skip to content

Convert MessageBuilder.disable_errors to a context manager - #10569

Merged
hauntsaninja merged 7 commits into
python:masterfrom
ikonst:disable_errors
Jun 2, 2021
Merged

hauntsaninja merged 7 commits into
python:masterfrom
ikonst:disable_errors

Conversation

@ikonst

@ikonst ikonst commented Jun 1, 2021

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread mypy/messages.py Outdated

def disable_errors(self) -> None:
@contextmanager
def disable_errors(self) -> ContextManager[None]:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return type should be Iterable[None] (or something with Generator). The function returns an iterable, the decorator turns it into a ContextManager.

@ikonst ikonst Jun 1, 2021 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, thanks. I guess I was trying to placate PyCharm which complained that it doesn't have __enter__() when I tried to use it "manually" within analyze_type_type_member_access.

Maybe I should just bite the bullet and use with in analyze_type_type_member_access (and indent all the code)?

Comment thread mypy/checkexpr.py
if right_map is None:
self.msg.disable_errors()
try:
# If right_map is None then we know mypy considers the right branch

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, the original comment added in #3666 got shifted around over the years.

Comment thread mypy/checkexpr.py Outdated
Comment thread mypy/nullcontext.py
def nullcontext() -> Iterator[None]:
yield
else:
from contextlib import nullcontext as nullcontext, contextmanager # noqa: F401

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: don't think you need to import contextmanager again. I think you could also move this to mypy/util.py instead of making a separate file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, copy-pasta, will fix.

Re mypy/util.py, I followed the pattern put in place by mypy/ordered_dict.py. Maybe we want to put both of those in mypy/backports.py or mypy/compat.py?

@hauntsaninja
hauntsaninja merged commit 28718fa into python:master Jun 2, 2021
@hauntsaninja

Copy link
Copy Markdown
Collaborator

Thank you!

@ikonst
ikonst deleted the disable_errors branch June 2, 2021 02:01
@atakiar

atakiar commented Jun 21, 2021

Copy link
Copy Markdown
Contributor

@ikonst we might be able to close #1184? Also PRs #9701 and #10251

@ikonst

ikonst commented Jun 21, 2021

Copy link
Copy Markdown
Contributor Author

Oops, I didn't notice there were other PRs and issues! Perhaps an admin can close them.

@JelleZijlstra

Copy link
Copy Markdown
Member

I closed the other two PRs. I think the issue can remain open for now since there are other similar refactors to be done (e.g., #10685).

JukkaL pushed a commit that referenced this pull request Sep 6, 2021
Related issue: #1184
Follows up to #10569, #10685

This PR:
* Refactors `Scope`
  - Replaces `Scope.enter_file` with `Scope.module_scope`
  - Replaces `Scope.enter_function` with `Scope.function_scope`
  - Splits `Scope.leave()` into corresponding context manager
* Deletes unused files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants