Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Checks: Fix E722
do not use bare except
#3748base: main
Are you sure you want to change the base?
Checks: Fix E722
do not use bare except
#3748Changes from 8 commits
2b60b99
ad41ff7
b444ea6
f0bfc76
c646724
70512e1
f66788a
bce6eeb
9b3b261
21b4199
0f5a689
cf4f4b1
a568514
42358fa
4636a1b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
The Log class has
self.parent.SetStatusText(text_string.strip())
and I think this is wx.Frame method. Since I have no idea what exceptions to expect from WxPython method calls, I will just use Exception.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.
This will be pylint warning raise broad-exception-raised / W0719. Is there way to avoid this?
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.
I'm not sure what you mean here. The exception Exception is not raised here. How this is different from the other cases?
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.
The problem is not flake8 W605, but pylint W0716 (broad-exception-raised). Just wondering if you have any suggestions for something different. Keeping
except Exception:
here.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.
Yes,
except Exception
causes Pylint's broad-exception-caught, but that's the issue everywhere, not just here, that was my point (and that its broad-exception-caught not broad-exception-raised).Anyway, I agree that we should be addressing Pylint exceptions as well. There is no point in replacing all
except:
byexcept Exception:
because we just exchange one issue for another, but we can make use of the fact that we can resolve the Pylint issue later and get the more clear cases now and leave the difficult cases for later.For this specific line, my whole issue was just if you point out this as something special, just an example, or something you want to address sooner than the other cases.
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.
This is just a sample. You mentioned previously that "except Exception" will silence flake8, but Pylint would have a problem with it, so fixing simple ones has been objective. Just wondering if we should deal with this now or later. I guess we'll address this when we try to fix Pylint problems.
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.
If it is difficult to figure out, let's deal with that later.