ArrayInterface.axes for ReshapedReinterpretArray #210
Conversation
Codecov Report
@@ Coverage Diff @@
## master #210 +/- ##
==========================================
+ Coverage 86.31% 86.78% +0.46%
==========================================
Files 11 11
Lines 1812 1808 -4
==========================================
+ Hits 1564 1569 +5
+ Misses 248 239 -9
Continue to review full report at Codecov.
|
This specializes `axes` for ReshapedReinterpretArray, and streamlines a bit of the code surrounding this type.
|
Is this ready for review |
chriselrod
left a comment
chriselrod
left a comment
There was a problem hiding this comment.
I guess this library has transitioned from tabs = 2 -> tabs = 4 spaces?
Yes. It doesn't implement anything for non-reshaped ReinterpretArray (in a sense, that's the harder case), and I added a
Not sure if you mean they were tabs before and spaces now (sorry, if I didn't notice and perhaps I should have updated my definition of tab), or if just the default indent had changed. Mostly I found the mix awkward and the 4-space indent dominates, so I went with that. |
| @test @inferred(ArrayInterface.strides(u_view_reinterpreted)) == (4,) | ||
| @test @inferred(ArrayInterface.strides(u_view_reshaped)) == (4, 4) | ||
|
|
||
| @test_broken @inferred(ArrayInterface.axes(u_vectors)) isa ArrayInterface.axes_types(u_vectors) |
There was a problem hiding this comment.
I take it that inference fails here?
There was a problem hiding this comment.
No, it's that it isn't of the type returned by axes_types.
There was a problem hiding this comment.
I need to do some clean up on axes and can fix this. I think we need to just go all in on OptionallyStaticUnitRange here.
There was a problem hiding this comment.
No worries, and no hurry (there's nothing wrong with a broken test, it's essentially a TODO list).
There was a problem hiding this comment.
Did you want me to fix this before this PR went forward or afterwards. Just want to make sure this PR isn't stalled because of me.
|
@timholy, is this ready to be merged? |
…#222) * +axis support methods from base and +tests * Get rid of some no good very bad spaghetti code. * Document examples of how to use axes here. * Incorporate ReinterpretArray changes from ArrayInterface.axes for ReshapedReinterpretArray #210. Don't assume we can go to parent arrays anymore because there are too many assumptions (offsets, sizing, views, etc) that can change it. * Document `SoneTo`
|
Just checking, this effectively got merged in #222, right? So close? |
|
It incorporates the same tests so unless there was something else needed here this should be good to close |
This specializes
axesfor ReshapedReinterpretArray, and streamlines abit of the code surrounding this type.