-
Notifications
You must be signed in to change notification settings - Fork 107
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
Use step plot for rate vectors #8671
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8671 +/- ##
==========================================
- Coverage 90.90% 90.90% -0.01%
==========================================
Files 342 343 +1
Lines 21207 21271 +64
==========================================
+ Hits 19279 19337 +58
- Misses 1928 1934 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
An idea is to use a summary key generator with hypothesis and prove that the resdata C-code and this Python-code is doing exactly the same. |
2d2dc47
to
1eff2db
Compare
@@ -45,11 +46,13 @@ def plot( | |||
plot_context.deactivateDateSupport() | |||
plot_context.x_axis = plot_context.INDEX_AXIS | |||
|
|||
draw_style = "steps-pre" if is_rate(plot_context.key()) else 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.
I tried changing steps-pre
to something non-sensical to check if it would fail any tests. At least not any tests in tests/unit_tests/gui
failed. Could you have a check?
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.
Apparently there is no "rate" plot tests. Maybe I can add one that will see if draw_style was set?
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.
at least there should be a test that triggers the line to uncover blatant mistakes in e.g. the string arguments given to matplotlib
This adds a key util function is_rate which determines whether the key represents a rate or not (Source: resdata). Additionally, adding tests that validate that the is_rate gives consinsent results with Summary.is_rate function.
- exploits "steps-pre" drawing style - tests that draw_style is set correctly
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.
👍🏻
Issue
Resolves #8574
Approach
If ensemble is rate plot it via
draw_style="steps"
(Screenshot of new behavior in GUI if applicable)
When applicable