BlockArrays.jl
BlockArrays.jl copied to clipboard
Add `findblockindex` support for `AbstractArray`
If you agree with this idea then I'll add tests.
Codecov Report
Attention: Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.
Project coverage is 95.60%. Comparing base (
5a72c40) to head (a34ba1f).
| Files | Patch % | Lines |
|---|---|---|
| src/blockaxis.jl | 0.00% | 8 Missing :warning: |
| src/abstractblockarray.jl | 0.00% | 3 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #345 +/- ##
==========================================
- Coverage 96.25% 95.60% -0.65%
==========================================
Files 18 18
Lines 1628 1639 +11
==========================================
Hits 1567 1567
- Misses 61 72 +11
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Seems good to me, but it'll be better to not use Base.IteratorsMD.flatten, which is an internal function.
to not use
Base.IteratorsMD.flatten
Do you have an alternative that concats tuples?
Perhaps foldl((x,y) -> (x...,y...), tuple_of_tuples)?
This PR actually exemplifies the point I was trying to make in #336. It leads to this inconsistency:
julia> using BlockArrays
julia> a1 = BlockArrays._BlockedUnitRange(2, [3, 6])
2-blocked 5-element BlockedUnitRange{Vector{Int64}}:
2
3
─
4
5
6
julia> a2 = mortar([2:3, 4:6])
2-blocked 5-element BlockVector{Int64, Vector{UnitRange{Int64}}, Tuple{BlockedUnitRange{Vector{Int64}}}}:
2
3
─
4
5
6
julia> findblockindex(a1, 4)
Block(2)[1]
julia> findblockindex(a1, (4,))
Block(2)[2]
julia> findblockindex(a2, (4,))
Block(2)[2]
i.e. it is not clear if findblock[index](a, k) is supposed to treat k as an index into the collection or a value of the collection, and there seems to be an inconsistency depending on what is input.
Another proposal for a "spelling" of this kind of operation (converting from a cartesian index to a block index) would be #346.
With that proposal, instead of using findblockindex(a1, (4,)) == Block(2)[2], you would use BlockIndices(a1)[4] == Block(2)[2]. I don't think the functionality of converting from a cartesian index to a block index should be called find*, since find*(a, k) in Base refers to finding the index of a value k in a collection a (those only happen to correspond if the collection is a unit range starting at 1).
It's the name of findblockindex that is confusing. This PR is intended to implement to_blockindex instead, which is analogue to to_indices. to_blockindices is a better analogy but I haven't considered how that will work.