-
Notifications
You must be signed in to change notification settings - Fork 32
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
Adapt to be used directly with fmu-ensemble #207
Conversation
93270db
to
620f5a0
Compare
76247d2
to
199f296
Compare
Codecov Report
@@ Coverage Diff @@
## master #207 +/- ##
==========================================
+ Coverage 91.29% 91.40% +0.10%
==========================================
Files 22 22
Lines 2781 2804 +23
==========================================
+ Hits 2539 2563 +24
+ Misses 242 241 -1
Continue to review full report at Codecov.
|
This is done to later be able to change the API, to obtain cleaner code and more API predictability. Some refactoring for code readability (pylint)
This function is only used once, and is better left by directly calling the Pandas API by the caller.
1b3e5f4
to
548b56c
Compare
ecl2df/summary.py
Outdated
@@ -86,7 +111,7 @@ def resample_smry_dates( | |||
Options for timeresampling are | |||
'daily', 'monthly' and 'yearly'. | |||
'last' will give out the last date (maximum), |
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.
Document first
as well?
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.
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.
Technically this looks good to me. Can't really comment on the functional. Maybe you want a second pair of eyes?
Thanks! I'll see if @asnyv has any further comments before merge. |
I think it looks ok @berland |
Based on what is needed to pass tests in fmu-ensemble
Necessary for able equinor/fmu-ensemble#166
Bonus changes:
df()