Making AxisArrays compatible with new core traits. #33
Conversation
| @@ -27,6 +27,10 @@ export # types | |||
| timeaxis, | |||
| timedim | |||
|
|
|||
| ImageCore.HasDimNames(::Type{A}) where {A<:AxisArray} = HasDimNames{true}() | |||
| ImageCore.namedaxes(a::AxisArray) = NamedTuple{axisnames(a)}(axisvalues(a)) | |||
| Base.names(a::AxisArray) = axisnames(a) | |||
There was a problem hiding this comment.
This is kind of iffy but it is necessary to really fulfill the intended meaning of HasDimNames.
There was a problem hiding this comment.
Remind me why the name of this function needs to be names? (Link to other package, for example?) The reason I ask:
julia> t = (x = 1, y = 2)
(x = 1, y = 2)
julia> typeof(t)
NamedTuple{(:x, :y),Tuple{Int64,Int64}}
julia> names(t)
ERROR: MethodError: no method matching names(::NamedTuple{(:x, :y),Tuple{Int64,Int64}})
Closest candidates are:
names(::Module; all, imported) at reflection.jl:98
Stacktrace:
[1] top-level scope at REPL[3]:1struct, for example, uses fieldnames rather than just names. axisnames seems like a really good name for this.
There was a problem hiding this comment.
It's what NamedDims uses to access the axis names https://github.com/invenia/NamedDims.jl/blob/master/src/wrapper_array.jl#L77.
But hadn't even considered using a different name. axisnames would certainly be more descriptive.
There was a problem hiding this comment.
Let's see what happens in invenia/NamedDims.jl#22
Codecov Report
@@ Coverage Diff @@
## master #33 +/- ##
==========================================
- Coverage 81.95% 81.61% -0.34%
==========================================
Files 1 1
Lines 133 136 +3
==========================================
+ Hits 109 111 +2
- Misses 24 25 +1
Continue to review full report at Codecov.
|
The goal here is to make AxisArrays compatible with the new traits added to ImageCore. This adds 3 methods to AxisArrays.