Skip to content

Enforce ruff/tryceratops rules (TRY)#266

Merged
jaraco merged 4 commits intopypa:mainfrom
DimitriPapadopoulos:TRY
Aug 26, 2024
Merged

Enforce ruff/tryceratops rules (TRY)#266
jaraco merged 4 commits intopypa:mainfrom
DimitriPapadopoulos:TRY

Conversation

@DimitriPapadopoulos
Copy link
Contributor

@DimitriPapadopoulos DimitriPapadopoulos commented Jun 29, 2024

I have not applied this one:

TRY003 Avoid specifying long messages outside the exception class

I believe it changes too much code, so I have left it out. I am happy to apply the TRY003 rule as well if you want me too.

Comment on lines +148 to +149
else:
return (dst, 1)
Copy link
Member

Choose a reason for hiding this comment

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

Honestly, I feel like TRY is overreaching here. There's no way that return (dst, 1) is going to cause an OSError, and re-writing with the else clause just distracts from the typical control flow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general, this rule might help identify cases where it's not clear which part of the code may throw. The usefulness of the TRY300 rule depends on the context: number of lines and complexity of the code under try.

In this specific case, it is quite clear that the return line won't raise en exception. On the other hand, I do not find the else clause less readable. The typical control flow is important of course, but so is clarity about what is tested under try. Additionally, the error control flow feels important too, enough to have 3 lines of comments.

TRY300 Consider moving this statement to an `else` block
TRY301 Abstract `raise` to an inner function
@jaraco jaraco merged commit d42a6aa into pypa:main Aug 26, 2024
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.

2 participants