Fix MultiIndex error in compute_forward_displacement (#794)#799
Fix MultiIndex error in compute_forward_displacement (#794)#799sfmig merged 7 commits intoneuroinformatics-unit:mainfrom
Conversation
…s-unit#794) Replace reindex(data.coords) with reindex_like(data) to avoid RuntimeError when DataArray coordinates have the _no_setting_name flag from pandas MultiIndex levels. - Fix _compute_forward_displacement (line 129) - Fix compute_displacement (line 115, deprecated) - Add regression test test_forward_displacement_with_multiindex_coords
There was a problem hiding this comment.
Pull request overview
Fixes a RuntimeError in compute_forward_displacement (and the deprecated compute_displacement) when the input xarray.DataArray originates from a pandas MultiIndex (e.g., via Series.to_xarray()), by switching to reindex_like() to avoid index-level name mutation.
Changes:
- Replace
reindex(data.coords)withreindex_like(data)in_compute_forward_displacement. - Apply the same fix in the deprecated
compute_displacementpath. - Add a regression unit test covering MultiIndex-derived coordinate inputs (issue #794).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
movement/kinematics/kinematics.py |
Uses reindex_like(data, fill_value=0) to align diff results back to the original array without triggering MultiIndex level-name mutation. |
tests/test_unit/test_kinematics/test_kinematics.py |
Adds a regression test ensuring compute_forward_displacement works with DataArrays created from a pandas MultiIndex via .to_xarray(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #799 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 36 36
Lines 2205 2205
=========================================
Hits 2205 2205 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
95a8873 to
220314a
Compare
This commit reverts accidental changes to documentation and configuration files, bringing them back to the state of the current main branch. Files restored: - CONTRIBUTING.md - docs/source/* - pyproject.toml - tests/test_unit/test_filtering.py - etc.
220314a to
0f5e6a5
Compare
|
Hey @sfmig ,Thank you for the feedback. I apologize for the accidental inclusion of unrelated documentation changes. I have updated the PR to strictly focus on the MultiIndex bug fix and its accompanying regression test... |
|
sfmig
left a comment
There was a problem hiding this comment.
Hi @Tushar7012 thanks for this and apologies for the slow response!
I think this is good to go now, I just rephrased a bit the docstring in the test for clarity.
1b9d294



Replace
reindex(data.coords)withreindex_like(data)to avoid a RuntimeError whenDataArraycoordinates originate from pandasMultiIndexlevels that carry the_no_setting_nameflag._compute_forward_displacement(line 129)compute_displacement(line 115, deprecated)test_forward_displacement_with_multiindex_coordsBefore submitting a pull request (PR), please read the contributing guide.
Description
What is this PR
Why is this PR needed?
When
compute_forward_displacementis called with an xarrayDataArraycreated from a pandasMultiIndex(viato_xarray()), the current implementation fails with aRuntimeError.This happens because
reindex(data.coords)internally attempts to rename index levels, which pandas explicitly disallows forMultiIndexobjects with the_no_setting_nameflag.As a result, valid inputs can unexpectedly fail even though the underlying data is correct.
What does this PR do?
This PR replaces the use of
reindex(data.coords, fill_value=0)withreindex_like(data, fill_value=0)in both the active and deprecated code paths.reindex_like()provides the same alignment and fill behavior while avoiding index-level name mutation, making the function safe for MultiIndex-backed inputs. A regression test has also been added to ensure this case remains supported going forward.References
compute_forward_displacement#794How has this PR been tested?
test_forward_displacement_with_multiindex_coords) based on the reproducible example provided in the issueIs this a breaking change?
No. This change preserves existing behavior and only affects internal reindexing logic to improve robustness for specific input types.
Does this PR require an update to the documentation?
No. This is an internal bug fix and does not change the public API or user-facing behavior.
Checklist