-
Notifications
You must be signed in to change notification settings - Fork 41
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
Implement a default offset option for all higher dimensions, to support a more complete centered
#340
Conversation
One concern that I see is that a lot of code uses |
Yes, exactly.
Though couldn't OffsetArrays provide an implementation of that? |
|
Seems I have a StackOverflow issue in one of the constructors. Would it be better as a kwarg? |
I'm a bit unsure of this PR's implications, e.g. julia> v = OffsetVector(1:2, (0,), 3)
1:2 with indices 1:2
julia> v'
1×2 adjoint(OffsetArray(::UnitRange{Int64}, 1:2)) with eltype Int64 with indices 4:4×1:2:
1 2
julia> view(v,:)'
1×2 adjoint(view(OffsetArray(::UnitRange{Int64}, 1:2), :)) with eltype Int64 with indices Base.OneTo(1)×OffsetArrays.IdOffsetRange(values=1:2, indices=1:2):
1 2
julia> v == view(v,:)
true
julia> v' == view(v,:)'
false so wrappers will behave differently as they are not inheriting the trailing axes of the parent |
Hmm, so it's using the So there could be an Although views of OffsetArrays seem to behave inconsisently?
Why does |
This is because the axes of an indexing operation are the axes of the indices, and a |
Oy. Ok. Given how tangled this is... maybe the easiest solution is to take defensive measures in the ImageFiltering functions. |
Closing, as this doesn't seem like something that we'll do here. |
Fixes #339 , but requires support from Julia Base to call the right
axes
methods. The API for this is probably not right, and obviously needs tests and docs. But would like feedback first.