-
Notifications
You must be signed in to change notification settings - Fork 57
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
error handling on mphys pygeo 'type' #264
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #264 +/- ##
==========================================
- Coverage 67.02% 67.01% -0.01%
==========================================
Files 47 47
Lines 12326 12327 +1
==========================================
Hits 8261 8261
- Misses 4065 4066 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 looks good, but what if we cast info["type"]
to lower case by default, maybe here? Not sure if that reflect best coding practices though
We could certainly do that. I don't know if that is the best spot to cast to lower case because I think using DVGeoMulti may bypass that. I just figured that the |
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.
Why not check the equality using info["type"].lower()
? That way it's case insensitive.
Purpose
This places an else check on the info['type'] because I kept passing in
type
keyword argument as uppercase 'FFD' when it was looking for lowercase 'ffd'. Now at least the warning message will tell the user what is breaking.Expected time until merged
Relatively simple so maybe a few days?
Type of change
Testing
I ran it locally passing in 'FFD' and it returned an error instead of proceeding.
Checklist
flake8
andblack
to make sure the Python code adheres to PEP-8 and is consistently formattedfprettify
or C/C++ code withclang-format
as applicable