-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reductions preserve the AxisArray wrapper (fixes #55) #56
Conversation
CC @tknopp. |
not sure if I am actually able reviewing this but my understanding is that you hook into special methods (i.e. The tests look good! |
67c306e
to
4f1da91
Compare
4f1da91 may be a deal-breaker. |
4f1da91
to
f7e2853
Compare
f7e2853
to
a4492b0
Compare
Small fix made, rebased, and inference issue filed: JuliaLang/julia#20714. I use |
@timholy: This comes up very frequently and I still do not understand what the correct workaround is. Even the type unstable form So the question is: How do I get the maximum intensity projection of a 3D AxisArray and I want the result to be still an AxisArray |
Try this: function fixaxes(A, axs)
ls = map(length, indices(A))
AxisArray(A, map((i,l)->length(i) == l ? i : i(Base.OneTo(1)), axs, ls))
end
julia> A = AxisArray(collect(reshape(1:15,3,5)), :y, :x)
2-dimensional AxisArray{Int64,2,...} with axes:
:y, Base.OneTo(3)
:x, Base.OneTo(5)
And data, a 3×5 Array{Int64,2}:
1 4 7 10 13
2 5 8 11 14
3 6 9 12 15
julia> fixaxes(maximum(parent(A), 1), axes(A))
2-dimensional AxisArray{Int64,2,...} with axes:
:y, Base.OneTo(1)
:x, Base.OneTo(5)
And data, a 1×5 Array{Int64,2}:
3 6 9 12 15 The |
My issue is more fundamental
|
If you do that to That said, I didn't realize that was the problem. Maybe the same as #62? I'd forgotten about that one. |
Sorry for mixing things. As you said its two independent issues. But from a higher level its simply a regression on the previous Images.jl workflow. The parent thing does not work. But I open a dedicated issue with testcase. |
If this is something that differs between 0.5.0 and 0.5.1, is it worth testing 0.5.0 as a separate matrix entry on travis, given not everyone has upgraded right away to 0.5.1? |
I didn't realize you do could that. See https://travis-ci.org/JuliaArrays/AxisArrays.jl/builds/215065371. I think I worked around the problem in a later commit, so I edited the comment above. (Given that it's passing on 0.5.0, I don't think it's worth adding to the test matrix, but others might feel differently.) |
Very curious result if you say@code_warntype AxisArrays.reduced_indices(axes(A), (1,))
. That's why some of the@inferred
are commented out.But otherwise this seems to work well.
Oh, argh! I'm runningrelease-0.5
and this tests on0.5.0
. This is a problem...