Compare slice_key array instead of value for use with 4+D data #7879
Conversation
|
@psobolewskiPhD this fixes it for me. Can you test? |
|
Yes! 🙏 |
There was a problem hiding this comment.
I think this is correct based on poking around the slice_key code.
slice_key stores values of the the non-displayed dims, so for 3D array, slice_key is just a single value array and the comparison is easy. But for 4D+, its multiple values, so need to do an array comparison.
I will approve, but would be good to get @jni and/or @Czaki to take a look too!
|
OOO! a novel windows seg fault! It does appear unrelated though, so we can rerun once it allows us. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7879 +/- ##
==========================================
+ Coverage 92.89% 92.93% +0.04%
==========================================
Files 641 641
Lines 60366 60385 +19
==========================================
+ Hits 56077 56121 +44
+ Misses 4289 4264 -25 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
based on how I had to set ShapeList.slice_key prior to adding the shapes for the tests to pass, I'm wondering if this is indicative of bad design. Couldn't slice_key be empty if the data is in 2D? |
it is, we've had many discussions about how we need to remove slicing information from the layers. But definitely out of scope for this PR! 😂 |
|
Thank you VERY MUCH @TimMonko for the fast fix, I first saw Peter's message about the failure on Zulip and I was like, omg if it's in 0.5.6 this is definitely going in 0.6.1 😂 but it will indeed be super-nice to have it in 0.6.0! 🚀 |
References and relevant issues
Closes #7878
Description
Compare array rather than value because slice_key[n] is array when greater than 3D.
Peter summarizes the issue/fix, nicely (thanks! I was scrambling between things)