Open mkitti opened 1 year ago
Merging #318 (2945d61) into master (08dc371) will decrease coverage by
1.29%
. The diff coverage is50.00%
.
@@ Coverage Diff @@
## master #318 +/- ##
==========================================
- Coverage 96.45% 95.16% -1.30%
==========================================
Files 5 5
Lines 451 434 -17
==========================================
- Hits 435 413 -22
- Misses 16 21 +5
Impacted Files | Coverage Δ | |
---|---|---|
src/axes.jl | 97.61% <50.00%> (-2.39%) |
:arrow_down: |
src/utils.jl | 96.00% <0.00%> (-2.00%) |
:arrow_down: |
src/OffsetArrays.jl | 97.50% <0.00%> (-0.80%) |
:arrow_down: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
Looking at the example from #316,
julia> Base.@kwdef struct MyIdOffsetRange
values::UnitRange
indices::UnitRange
end
MyIdOffsetRange
julia> x = MyIdOffsetRange(values=5:8, indices=5:8)
MyIdOffsetRange(5:8, 5:8)
julia> x.values
5:8
julia> x.values === x
false
In this example, the returned results correspond exactly to the displayed ones, as both directly read the fields of the struct. However, for an IdOffsetRange
, we obtain another IdOffsetRange
, which isn't what was displayed.
julia> id
OffsetArrays.IdOffsetRange(values=5:8, indices=6:9)
julia> id.values
OffsetArrays.IdOffsetRange(values=5:8, indices=6:9)
julia> id.values === id
true
This isn't an objection to the PR, but I wonder if we need to be consistent here?
The consistency choice I made here was for
id.values === values(id)
values
is part of the API already.
If you really wanted a UnitRange, this could work:
UnitRange(id.values)
For that reason, I added it to the documentation.
This is a full implementation of #316. Virtual properties for
values
andindices
are added and documented.This differs from the proposed implementation in #316 in that the properties return
IdOffsetArray
rather thanUnitRanges
.Another discovered benefit is the ability to forward the properties to construct another
IdOffsetRange
.