-
Notifications
You must be signed in to change notification settings - Fork 229
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
misc: Revamp exception hierarchy #2500
base: master
Are you sure you want to change the base?
Conversation
df6e14c
to
a104b48
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2500 +/- ##
==========================================
- Coverage 87.29% 87.28% -0.01%
==========================================
Files 238 238
Lines 45609 45609
Branches 4042 4047 +5
==========================================
- Hits 39814 39811 -3
- Misses 5112 5115 +3
Partials 683 683 ☔ View full report in Codecov by Sentry. |
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.
Looks fine!
Let me know if I'm being too picky! |
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.
LGTM
devito/operator/operator.py
Outdated
@@ -188,7 +189,8 @@ def _sanitize_exprs(cls, expressions, **kwargs): | |||
|
|||
for i in expressions: | |||
if not isinstance(i, Evaluable): | |||
raise InvalidOperator("`%s` is not an `Evaluable` object" % str(i)) | |||
raise CompilationError(f"`{i!s}` is not an Evaluable object, " | |||
"check again your Equation") |
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.
Nitpick: "check your equation again" would be more natural phrasing, and the comma should probably be a semicolon
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.
fixing now
"than available (%s) " % humanbytes(available_mem) + | ||
"on physical device, this will start swapping") | ||
raise MemoryError( | ||
f"Trying to allocate more memory ({humanbytes(required_mem)}) " |
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.
Honestly much better.
5351bff
to
ab90464
Compare
ab90464
to
140a1f5
Compare
This was long due
Also improves handling of C-level errors