Set levels in index #18
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #18 +/- ##
==========================================
+ Coverage 96.96% 96.98% +0.01%
==========================================
Files 27 27
Lines 1319 1358 +39
Branches 146 153 +7
==========================================
+ Hits 1279 1317 +38
Misses 23 23
- Partials 17 18 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| # TODO mypy says this is unreachable | ||
| if not isinstance(ini, pd.MultiIndex): | ||
| msg = f"Expected MultiIndex, got {type(ini)}" | ||
| raise TypeError(msg) |
There was a problem hiding this comment.
Ah yes that's because the type hint for ini is pd.MultiIndex. You'll probably need to push this check one layer higher into the function that operates on DataFrames (where the index could be just index, rather than a multi-index)
There was a problem hiding this comment.
Can be resolved I think?
| msg = f"Expected MultiIndex, got {type(ini)}" | ||
| raise TypeError(msg) | ||
|
|
||
| df = ini.to_frame(index=False) |
There was a problem hiding this comment.
I don't really know why, but calling to_frame is crazy expensive. Try and use https://github.com/openscm/gcages/blob/69452181c365c45f83d59edc3189727f651e5655/src/gcages/index_manipulation.py#L18 and
to_frame. We can talk Monday afternoon if it's still not clear (which is very likely going to be the case as this multi-index stuff is super confusing).
There was a problem hiding this comment.
Can be resolved I think?
znichollscr
left a comment
znichollscr
left a comment
There was a problem hiding this comment.
Super nice. Some small comments and clean up then good to merge.
| ValueError | ||
| If the length of the values is not equal to the length of the index | ||
| Examples |
There was a problem hiding this comment.
This thread can be resolved I assume ?
Co-authored-by: znichollscr <114576287+znichollscr@users.noreply.github.com>
|
Thank you for the comments. I had to make some changes to make mypy happy (see last commit). It seems not ideal to convert a numpy array to a list only to satisfy the mypy checks. Or maybe that's the point of it? If you think that's a minor issue, we can also merge. |
Fix up codes type confusion
|
Merge when you're happy @crdanielbusch |
Description
Add a function to set one or more levels in a MultiIndex.
Checklist
Please confirm that this pull request has done the following:
changelog/