JuliaArrays / OffsetArrays.jl

Fortran-like arrays with arbitrary, zero or negative starting indices.
Other
195 stars 40 forks source link

[Feature Request] Add values and indices virtual properties to IdOffsetRange #316

Open mkitti opened 1 year ago

mkitti commented 1 year ago

Currently when you show an IdOffsetRange it displays indices and values:

julia> a = OffsetArrays.IdOffsetRange(1:4, 4)
OffsetArrays.IdOffsetRange(values=5:8, indices=5:8)

The intuition from this display is that an IdOffsetRange might have a values and indices property. However, this is not the case.

julia> a.values
ERROR: type IdOffsetRange has no field values
Stacktrace:
 [1] getproperty(x::OffsetArrays.IdOffsetRange{Int64, UnitRange{Int64}}, f::Symbol)
   @ Base ./Base.jl:38
 [2] top-level scope
   @ REPL[112]:1

julia> a.indices
ERROR: type IdOffsetRange has no field indices
Stacktrace:
 [1] getproperty(x::OffsetArrays.IdOffsetRange{Int64, UnitRange{Int64}}, f::Symbol)
   @ Base ./Base.jl:38
 [2] top-level scope
   @ REPL[113]:1

I suggest that we add those properties based on what is shown via show: https://github.com/JuliaArrays/OffsetArrays.jl/blob/08dc3711b88930e59a27d1b870415c394219e4e0/src/axes.jl#L278

An example implementation is as follows.

julia> Base.getproperty(r::IdOffsetRange, s::Symbol) = hasfield(typeof(r),s) ? getfield(r,s) : 
                                                       s == :values ? (first(r):last(r)) :
                                                       s == :indices ? (first(eachindex(r)) : last(eachindex(r))) :
                                                       error("type IdOffsetRange has no property $s")

julia> Base.propertynames(r::IdOffsetRange) = (fieldnames(typeof(r))..., :values, :indices)

julia> a.offset
4

julia> a.parent
1:4

julia> a.indices
5:8

julia> a.values
5:8
mkitti commented 1 year ago

One issue is the difference between Base.values and .values here:

julia> values(a)
IdOffsetRange(values=5:8, indices=5:8)

julia> a.values
5:8
jishnub commented 1 year ago

As i understand, the displayed form is meant to act as a constructor, and not to indicate properties. Are there other array types where property access corresponds to the displayed form?

mkitti commented 1 year ago

The default display for a struct type is to present a constructor syntax given it's fields.

julia> struct Foo
           x::Int
           y::Int
       end

julia> Foo(3,2)
Foo(3, 2)

The constructor itself appears like it was generated by Base.@kwdef where the keywords correspond to fields.

julia> Base.@kwdef struct MyIdOffsetRange
           values::UnitRange
           indices::UnitRange
       end
MyIdOffsetRange

julia> MyIdOffsetRange(values=5:8, indices=5:8)
MyIdOffsetRange(5:8, 5:8)

If the default display were as follows, I would understand more clearly that the fields correspond to the displayed arguments.

julia> Base.show(io::IO, r::IdOffsetRange) = print(io, IdOffsetRange, "(", r.parent, ", ", r.offset, ")")

julia> a = OffsetArrays.IdOffsetRange(1:4, 4)
IdOffsetRange(1:4, 4)

However, a deliberate choice was made to display the keyword form. It's frustrating that one cannot extract the values and indices shown directly based on that presentation.

mkitti commented 1 year ago

Happy holidays!

Thinking about this further, I think some additional documentation on how to extract the values and indices would go a long way. To that end, I've submitted https://github.com/JuliaArrays/OffsetArrays.jl/pull/317 .

In comparison, #318 seems to be quite involved to fix, although I still plan to do so.