Embrace pairs for show and construction

[timholy]

This changes the show method for IdOffsetRange to print as follows:

julia> using OffsetArrays: IdOffsetRange

julia> IdOffsetRange(3:5, 1)
(2 => 4):(4 => 6)

It allows you to construct them as displayed:

julia> for p in pairs( (0=>5):(2=>7) )
           println(p)
       end
0 => 5
1 => 6
2 => 7

The advantage is that this makes both the indexes and the values apparent.
It fixes the problem that two ranges with different indexes currently print the same:

julia> r1 = IdOffsetRange(0:2, 0)
OffsetArrays.IdOffsetRange(0:2)

julia> firstindex(r1)
1

julia> r2 = IdOffsetRange(Base.IdentityUnitRange(-1:1), 1)
OffsetArrays.IdOffsetRange(0:2)

julia> firstindex(r2)
0

[codecov]

Codecov Report

Merging #179 (42c8cf6) into master (42c8cf6) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #179   +/-   ##
=======================================
  Coverage   98.85%   98.85%           
=======================================
  Files           4        4           
  Lines         261      261           
=======================================
  Hits          258      258           
  Misses          3        3           

---

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 42c8cf6...b6f7d83. Read the comment docs.

[timholy]

Following the pattern in Julia itself, printing changes may not be considered breaking. OTOH, if we want to take advantage of the opportunity to fix

OffsetArrays.jl/src/axes.jl

Lines 56 to 74 in 42c8cf6

	!!! warning
	In the future, *conversion* will preserve both the values and
	the indices, throwing an error when this is not achievable. For instance,
	r = convert(OffsetArrays.IdOffsetRange{Int,UnitRange{Int}}, 3:4)
	has `r[1] == 3` and `r[2] == 4` and would satisfy `r == 3:4`, whereas
	```julia
	julia> convert(OffsetArrays.IdOffsetRange{Int,Base.OneTo{Int}}, 3:4) # future behavior, not present behavior
	ERROR: ArgumentError: first element must be 1, got 3
	```
	where the error will arise because the result could not have the same axes as the input.
	An important corollary is that `typeof(r1)(r2)` and `oftype(r1, r2)` will behave differently:
	the first coerces `r2` to be of the type of `r1`, whereas the second converts.
	Developers are urged to future-proof their code by choosing the behavior appropriate for each usage.

then we could go to v2. (We'd want to decide #174 at the same time.)

[jishnub]

The constructor commits type-piracy though, although the method (::Pair):(::Pair) doesn't exist in Base currently.

[timholy]

Agreed, but personally I think it's worth it. "Fixing" a MethodError is a pretty weak form of piracy.

But perhaps we should move IdOffsetRange to Base, and have a stub here for previous Julia versions?

[johnnychen94]

It's not very clear to me how (2 => 4):(4 => 6) is interpreted until I see the implementation. Part of the reason here is that both : and => are read as "from ... to...".

I'm sitting tight to see how JuliaLang/julia#39069 is progressing

[timholy]

Interesting point. Any other ideas? From my standpoint, the current printing isn't really acceptable in the long run since it doesn't show the indices. So we have to do something. It would be ideal if it's repr-round-trippable, so you can copy/paste output to achieve input.

[mcabbott]

How about 1:10:Ref(0)? Still piracy of course but only contains 3 numbers not 4, which I think matches the degrees of freedom of the range. You could also print OneTo(3):Ref(0) when appropriate.

[timholy]

But that doesn't distinguish the indices from the values. The advantage here is that it's a range-of-pairs, index => value. The similar idea for a Dict would be Dict(('a'=>1):('z'=>26)).

[mcabbott]

The type only contains a range and an offset, so it seems a little confusing to me to enter it with a redundant 4th number, which if you get wrong gives an error (presumably). It could still be printed in some more informative way, maybe when compact=>false or whatever, which shows both?

[timholy]

Understood, but for reference, do you think it's weird to have a 4th value for the Dict example? How would you spell that in a nice way without including the 4th value?

[mcabbott]

The dictionary is Dict('a':'z' .=> 1:26), and I guess that's also an error if you get the lengths wrong... and your spelling sort-of broadcasts the : instead of the =>. This seems less weird as you are fundamentally making two vectors & passing them in. While the offset range is (in my head at least) a more unified object... I guess what seems odd is having (almost) syntax with redundancy.

A more boring option would just be to make this work, and print it:

julia> OffsetArray('a':'z', -10)
'a':1:'z' with indices -9:16

julia> OffsetArray('a':'z', -9:16)
'a':1:'z' with indices -9:16

julia> repr(ans)  # this should probably change too
"'a':1:'z' with indices -9:16"

julia> IdOffsetRange(1:26, -10)
OffsetArrays.IdOffsetRange(-9:16)

julia> IdOffsetRange(1:26, -9:16)
ERROR: MethodError: no method matching IdOffsetRange(::UnitRange{Int64}, ::UnitRange{Int64})

[johnnychen94]

If we're interpreting => as key => value pair, then I think 2:4 => 5:7 should be equivalent to (2 => 5):(4 => 7) instead of (2 => 4):(5 => 7)

Hence if we introduce this in Base, it should be something like

Base.:(:)(src::Pair, dest::Pair) = src.first:dest.first => src.second:dest.second

Forcing it to return an IdOffsetRange makes little sense to me as it breaks the 2:4 => 5:7 and (2 => 5):(4 => 7) equivalence by adding a new range meaning (IdOffsetRange) to it. I mean, it's as little sense as making (1, 1):(4, 4) to be CartesianIndex(2, 2):CartesianIndex(4, 4).

It might be better to adjust this PR to:

julia> IdOffsetRange(4:6, 1)
IdOffsetRange((2 => 5):(4 => 7))

[timholy]

This PR doesn't "do anything" with 2:4 => 5:7. A Pair-of-ranges seems like a well-defined concept. Conversely, a range-of-pairs currently is an error:

julia> 2:4 => 5:7    # no error
2:4 => 5:7

julia> (2=>5) : (4=>7)
ERROR: MethodError: no method matching -(::Pair{Int64, Int64}, ::Pair{Int64, Int64})
Stacktrace:
 [1] (::Colon)(start::Pair{Int64, Int64}, stop::Pair{Int64, Int64})
   @ Base ./range.jl:7
 [2] top-level scope
   @ REPL[1]:1

Consequently we have the opportunity to make it mean something. We could even rename IdOffsetRange to RangePair. They're "Id" (idempotent) only when the parent range starts at 1:

julia> r = OffsetArrays.IdOffsetRange(1:5, -3)
(-2 => -2):(2 => 2)

julia> axes(r)
((-2 => -2):(2 => 2),)

julia> r = OffsetArrays.IdOffsetRange(2:6, -3)
(-2 => -1):(2 => 3)

julia> axes(r)
((-2 => -2):(2 => 2),)

[timholy]

Since most of the complaints seem to be about the construction part, how about I rip that out and we just have the display? JuliaLang/julia#39069 has been closed, so I don't think there's any concern about conflict.

[jishnub]

I'm in favor of this as it is undoubtedly helpful, albeit a bit tricky to parse

[timholy]

We could also do

IdOffsetRange(vals=3:5, indices=-2:0)

and add that as a keyword-only constructor. LIke the Pairs implementation, it doesn't show any of the types but I think that's much less important (and arguably something we should de-emphasize as an internal matter).