Skip to content

Conversation

@jishnub
Copy link
Member

@jishnub jishnub commented Feb 9, 2021

Now

julia> a = OffsetVector(axes(OffsetVector(1:10, -5), 1), 5)
OffsetArrays.IdOffsetRange(-4:5) with indices 1:10

julia> axes(a, 1)
OffsetArrays.IdOffsetRange(1:10)

Fixes #198 (comment)

@codecov
Copy link

codecov bot commented Feb 9, 2021

Codecov Report

Merging #200 (837b6d4) into master (3a9b52f) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #200      +/-   ##
==========================================
- Coverage   98.25%   98.25%   -0.01%     
==========================================
  Files           5        5              
  Lines         287      286       -1     
==========================================
- Hits          282      281       -1     
  Misses          5        5              
Impacted Files Coverage Δ
src/OffsetArrays.jl 97.94% <100.00%> (-0.02%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a9b52f...837b6d4. Read the comment docs.

@jishnub
Copy link
Member Author

jishnub commented Feb 9, 2021

The present way of displaying OffsetRanges is a bit unintuitive, as It's not clear what the type of the object is. Looking at the output, the constructed object appears to be an AbstractRange, but it's not.

Also, conventionally the indices displayed correspond to the outermost wrapper, eg.

julia> ones(3:3, 4:4)
1×1 OffsetArray(::Matrix{Float64}, 3:3, 4:4) with eltype Float64 with indices 3:3×4:4:
 1.0

In the case of OffsetRanges, the outermost wrapper is not displayed, therefore at first glance the indices appear to correspond to the parent range. Copy-pasting the parent range from the display would also not let you obtain the correct type or axes.

Would it be better to display this instead as

julia> a = OffsetVector(axes(OffsetVector(1:10, -5), 1), 5)
OffsetVector(OffsetArrays.IdOffsetRange(-4:5), 1:10) with indices 1:10

This way the output may be copy-pasted as well to create an OffsetRange that is equal to a.

julia> b = OffsetVector(OffsetArrays.IdOffsetRange(-4:5), 1:10);

julia> b == a
true

@timholy
Copy link
Member

timholy commented Feb 10, 2021

Fundamentally we just have to work both the indices and values into IdOffsetRange printing (like #179 except not a solution that everyone hates 😄). Conveying the exact types of objects is nice, but conveying their value/behavior is vastly more important. I'd be fine if IdOffsetRange doesn't appear anywhere in the display as long as we know what the indices and values are.

@jishnub
Copy link
Member Author

jishnub commented Feb 10, 2021

After 837b6d4 the type of the parent array is not displayed anymore, just the values and the indices.

julia> a = OffsetVector(axes(OffsetVector(1:10, -5), 1), 5)
-4:5 with indices 1:10

@jishnub jishnub merged commit 5dd0717 into JuliaArrays:master Feb 11, 2021
@jishnub jishnub deleted the show branch February 11, 2021 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

offset IdOffsetRange not displayed correctly, and can't be used as offsets

2 participants