-
Notifications
You must be signed in to change notification settings - Fork 10
Remove any hardcoded reference to "month", "day" and "hour" #381
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
This reverts commit ceee06e.
dalonsoa
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.
The changes look totally sensible so I'm happy to move on with this one and marking the test as xfail if you're sure that the test is failing because it is not correctly setup and not because the changes to the functions you have made have broken the execution pipeline.
So we don't forget, also open an issue about it, assign it to me and add it to this iteration.
alexdewar
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.
Seems fine to me.
src/muse/outputs/mca.py
Outdated
| data_sector.append(a[a["consumption"] != 0]) | ||
|
|
||
| if len(data_sector) > 0: | ||
| output = pd.concat([u for u in data_sector], sort=True).reset_index() |
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.
Does this need to be a generator expression?
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #381 +/- ##
===========================================
- Coverage 71.37% 71.36% -0.01%
===========================================
Files 44 44
Lines 5928 5916 -12
Branches 1169 1162 -7
===========================================
- Hits 4231 4222 -9
+ Misses 1374 1371 -3
Partials 323 323 ☔ View full report in Codecov by Sentry. |
Description
After #392, there are still a couple of places in the code that referenced "month", "day" and "hour" by name, which could cause problems if the level names were changed from the default (which is possible to in the settings file). This started off as a PR to remove all of these hardcoded references, but this ended up having a few other implications, so this PR now addresses a few things.
Specific changes:
scipy_match_demandininvestments.py. This was easy to fix as it was just dropping the "month", "day" and "hour" columns needlessly, so I just removed this.sector_supplyandsector_consumptioninmca.pywhich prepare a couple of output files. I found a workaround to do a similar thing without referring to "month", "day" and "hour" by name, and in the process slightly modified the output files (for the better, I think).This second change then allowed
test_minimum_serviceto start actually testing something (see #382), but the test was failing because it was testing the wrong thing. So I fixed the test so it's now testing the right thing, although the test is still failing, presumably because the supply data that it's using is wrong (see #321, #335 and #368). So not sure what to do now. @dalonsoa If we're planning on fixing the broken supply output, are you happy for me to mark this test as xfail for now so we can merge this?Fixes #363
Fixes #382
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