-
Notifications
You must be signed in to change notification settings - Fork 203
Avoid raising bare Exception #1168
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
base: main
Are you sure you want to change the base?
Avoid raising bare Exception #1168
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1168 +/- ##
==========================================
+ Coverage 91.26% 91.29% +0.03%
==========================================
Files 261 262 +1
Lines 26877 26899 +22
==========================================
+ Hits 24529 24558 +29
+ Misses 2348 2341 -7 ☔ View full report in Codecov by Sentry. |
This seems like an improvement, thank you. It's really hard to catch a specific |
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 this! I've got a few minor comments but overall looks great!
… on exception messages)
Thanks for the support @kiri11 ! I hope everything is now ready (apart from that |
You're welcome! (although I didn't do anything) |
…belong, to its own file in _parser/_parsing_check.py * Group all exceptions in _exceptions.py * Remove now empty file _excep.py (now redundant with _exceptions.py)
Summary
Before tackling issue #457, there are some bare Exceptions thrown from within the code.
Here is the logic for changes in Exceptions:
ValueError
when issue caused by user inputCSTValidationError
&TypeError
when issue occurs during the CST validation processParserSyntaxError
when issue occurs during the parsing processCSTLogicError
to replace all Exception which contained "Logic error" in the cause messageI'm all for changes if some Exceptions changes are not OK,
In particular, there are some
ParserSyntaxError
which may not be meaningful - with raw_line=0, raw_column=0 paramsTest Plan
Keep the current test plan.