-
Notifications
You must be signed in to change notification settings - Fork 108
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
Fix an issue where summary dates are not precise enough #6897
Fix an issue where summary dates are not precise enough #6897
Conversation
50664cb
to
ed6e059
Compare
36ecd51
to
ae3c95d
Compare
ae3c95d
to
97dcacf
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #6897 +/- ##
==========================================
+ Coverage 83.76% 84.16% +0.39%
==========================================
Files 368 368
Lines 21660 21860 +200
Branches 948 911 -37
==========================================
+ Hits 18144 18398 +254
+ Misses 3222 3168 -54
Partials 294 294 ☔ View full report in Codecov by Sentry. |
acb42a8
to
8b27390
Compare
8b27390
to
68fc26d
Compare
second_character = draw(st.sampled_from("OWGVLPT")) | ||
third_character = draw(st.sampled_from("PIF")) | ||
fourth_character = draw(st.sampled_from("RT")) | ||
# local = draw(st.sampled_from(["", "L"])) if first_character in "BCW" else "" |
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.
Stray comment, something that is needed?
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.
Fixed now
try: | ||
date_unit = DateUnit[unit_key] | ||
except KeyError: | ||
raise ValueError(f"Unknown date unit in {spec}: {unit_key}") from None |
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 dont you want the context 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.
I think the keyerror is just misleading compared to the ValueError raised.
Circumvents equinor/resdata#906 by reimlementing the summary reading with resfo. This also makes for a performance increase for summaries with many keys, but slightly worse performance (15%) for summaries with many dates.
This also ensures that float32 values are not converted in any way which removes precision loss. This also means that snapshots/reference values need to be updated.
When applicable