Skip to content

SIM105: Replace try-except-pass blocks with the contextlib.suppress context manager - #3448

Closed
seisman wants to merge 2 commits into
mainfrom
SIM105
Closed

SIM105: Replace try-except-pass blocks with the contextlib.suppress context manager#3448
seisman wants to merge 2 commits into
mainfrom
SIM105

Conversation

@seisman

Copy link
Copy Markdown
Member

Description of proposed changes

Fix ruff SIM105 violations.

What it does

Checks for try-except-pass blocks that can be replaced with the contextlib.suppress context manager.

Why is this bad?

Using contextlib.suppress is more concise and directly communicates the intent of the code: to suppress a given exception.

Note that contextlib.suppress is slower than using try-except-pass directly. For performance-critical code, consider retaining the try-except-pass pattern.

Not sure why ruff doesn't catch these violations, but the codes look cleaner with contextlib.suppress.

@seismanseisman added maintenance Boring but important stuff for the core devs skip-changelog Skip adding Pull Request to changelog labels Sep 23, 2024
@seismanseisman added this to the 0.14.0 milestone Sep 23, 2024
@seismanseisman added the needs review This PR has higher priority and needs review. label Sep 23, 2024
@weiji14

Copy link
Copy Markdown
Member

Not sure why ruff doesn't catch these violations, but the codes look cleaner with contextlib.suppress.

Looks the the SIM105 rule won't work with >1 line under the try: block, see astral-sh/ruff#8593. The idea I think is that if the 1st line fails, it's not so clear that the 2nd line wouldn't run if there is not an except: below it, see also rationale at astral-sh/ruff#1947.

@seisman

Copy link
Copy Markdown
MemberAuthor

The idea I think is that if the 1st line fails, it's not so clear that the 2nd line wouldn't run if there is not an except: below it, see also rationale at astral-sh/ruff#1947.

Makes sense. Closing the PR.

@seismanseisman closed this Sep 24, 2024
@seisman
seisman deleted the SIM105 branch September 24, 2024 02:12
@seismanseisman removed the needs review This PR has higher priority and needs review. label Sep 24, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenanceBoring but important stuff for the core devsskip-changelogSkip adding Pull Request to changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@seisman@weiji14