Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #392 +/- ##
===========================================
+ Coverage 71.34% 71.37% +0.02%
===========================================
Files 44 44
Lines 5916 5928 +12
Branches 1168 1169 +1
===========================================
+ Hits 4221 4231 +10
- Misses 1373 1374 +1
- Partials 322 323 +1 ☔ View full report in Codecov by Sentry. |
dalonsoa
left a comment
dalonsoa
left a comment
There was a problem hiding this comment.
Given that you're on it, add a dedicated tests that checks the new function.
src/muse/timeslices.py
Outdated
| return convert_timeslice(DataArray([nhours]), timeslices).squeeze() | ||
|
|
||
|
|
||
| def drop_timeslice(data: DataArray): |
There was a problem hiding this comment.
Ideally, add output type:
| def drop_timeslice(data: DataArray): | |
| def drop_timeslice(data: DataArray) -> DataArray: |
alexdewar
left a comment
alexdewar
left a comment
There was a problem hiding this comment.
I've made a small suggestion (that probs won't work) but other than that, LGTM.
src/muse/timeslices.py
Outdated
| if "timeslice" not in data.dims: | ||
| return data | ||
|
|
||
| return data.drop_vars([c for c in data.timeslice.indexes]) |
There was a problem hiding this comment.
Would this work:
| return data.drop_vars([c for c in data.timeslice.indexes]) | |
| return data.drop_vars(data.timeslice.indexes) |
If not, how about:
| return data.drop_vars([c for c in data.timeslice.indexes]) | |
| return data.drop_vars(list(data.timeslice.indexes)) |
|
@dalonsoa Would you mind taking a quick look at this so I can merge it? |
dalonsoa
left a comment
dalonsoa
left a comment
There was a problem hiding this comment.
LGTM! Sorry, I didn't realized the changes had been implemented.
3 participants
Description
Adds a helper function for dropping the timeslice variable from data arrays, and uses this throughout the code instead of
drop_vars(["timeslice", "month", "day", "hour"])This partially fixes #363, but a few other changes will be required which I'm addressing in #381
Type of change
Please add a line in the relevant section of
CHANGELOG.md to
document the change (include PR #) - note reverse order of PR #s.
Key checklist
$ python -m pytest$ python -m sphinx -b html docs docs/buildFurther checks