-
Notifications
You must be signed in to change notification settings - Fork 190
fix: Use max of float-or-array fields for numerical metadata checks #2235
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2235 +/- ##
=======================================
Coverage 82.71% 82.71%
=======================================
Files 20 20
Lines 1608 1608
=======================================
Hits 1330 1330
Misses 278 278 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
effigies
left a comment
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 agree. Can you base this on maint/1.10.1 so that we can cut a patched schema release?
|
I'm happy to add explicit tests for |
|
Searching through
Would be good to grep for any numeric comparisons for each of these to see if we should wrap them in min/max. At least some EchoTime ones already check for |
|
I just searched for each of those in the |
|
@tsalo is this ready for review? |
|
It is. |
Closes #2234.
Changes proposed:
max(float)will return the float. The checks I modified arePostLabelingDelayGreater,BolusCutOffDelayTimeGreater,LabelingDurationGreater, andEchoTimeGreaterThan.