Closed LilithHafner closed 4 months ago
Attention: 3 lines
in your changes are missing coverage. Please review.
Comparison is base (
35221bd
) 73.92% compared to head (0a18438
) 73.88%.
Files | Patch % | Lines |
---|---|---|
src/show.jl | 25.00% | 3 Missing :warning: |
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Oh wow. I didn't realize we weren't testing this. That's supposed to exist for components that are multi-dimensional arrays of componentarrays. Which, maybe that's also just a thing that nobody needs.
In addition to being untested, I think that behavior has a broken constructor. Specifically, components that are multidimensional arrays of component arrays are automatically flattened to single dimensional arrays of component arrays.
A = fill(ComponentArray(x = 3), 2, 2);
B = fill(4, 2, 2)
C = ComponentArray(A = A, B = B);
(size(A), size(B), size(C.A), size(C.B)) === ((2, 2), (2, 2), (4,), (2, 2))
(eltype(A), eltype(B), eltype(C.A), eltype(C.B)) === (
ComponentVector{Int64, Vector{Int64}, Tuple{Axis{(x = 1,)}}},
Int64,
Any,
Int64)
Ah. Okay well if it's been broken this whole time and nobody's noticed it, then yeah, let's just get rid of it.
As I was reviewing the subtypes of
AbstractAxis
to try to get a better understanding of how ComponentArrays work, I couldn't figure out the point of the IdxMap type parameter in ShapedAxis, so I decided to remove it and see what breaks....But all the tests pass without this type parameter, so maybe we can just remove it? It seems to me that the index mapping is tracked at the level of the axis containing the ShapedAxis. For example, in
the indices of the matrix are tracked by the ViewAxis, not the ShapedAxis.
I changed the supertype from
AbstractAxis{NamedTuple()}
toAbstractAxis{nothing}
becausenothing
seems like a better indicator of "there's no index information available".(I also accidentally removed some trailing whitespace, happy to revert that NFC change if desired)