Fix axes and size of BlockIndices#490
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #490 +/- ##
=======================================
Coverage 94.52% 94.53%
=======================================
Files 19 19
Lines 1789 1810 +21
=======================================
+ Hits 1691 1711 +20
- Misses 98 99 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@dlfivefifty @jishnub I'll merge this tomorrow unless I hear back from you earlier, I think the definition is uncontroversial and it would be a nasty silent bug to hit. |
|
I merged it. I take the view that if (1) nothing downstream is broken and (2) you are confident the changes are good/correct; then please feel free to just merge and tag without review. We can always revert a change if it turns out to be a problem (which often involves adding another downstream test where the problem was detected). |
The current definition of
size(iter::BlockIndices)was assuming the indices were ranges (resulting from #483 whereBlockIndexRangewas generalized toBlockIndicesbut the definition ifsizewasn't updated accordingly), so for (non-contiguous) non-ranges it was giving the wrong result, this PR fixes that. Additionally, previously onlysizewas defined which meant that BlockIndices where the indices had non-trivial axes (like blocked axes) lost information, which is now fixed and tested.