broken show method for Base.LogicalIndex

[stevengj]

As reported in JuliaLang/IJulia.jl#892,

julia> Base.LogicalIndex([true])
1-element Base.LogicalIndex{Int64,Array{Bool,1}}:
Error showing value of type Base.LogicalIndex{Int64,Array{Bool,1}}:
ERROR: getindex not defined for Base.LogicalIndex{Int64,Array{Bool,1}}

stemming from an error in the text/plain method for show:

julia> show(stdout, Base.LogicalIndex([true]))
Base.LogicalIndex(Bool[1])
julia> show(stdout, "text/plain", Base.LogicalIndex([true]))
1-element Base.LogicalIndex{Int64,Array{Bool,1}}:
ERROR: getindex not defined for Base.LogicalIndex{Int64,Array{Bool,1}}
Stacktrace:
 [1] error(::String, ::Type) at ./error.jl:42
 [2] error_if_canonical_getindex(::IndexCartesian, ::Base.LogicalIndex{Int64,Array{Bool,1}}, ::Int64) at ./abstractarray.jl:991
 [3] _getindex at ./abstractarray.jl:980 [inlined]
 [4] getindex at ./abstractarray.jl:981 [inlined]
 [5] isassigned(::Base.LogicalIndex{Int64,Array{Bool,1}}, ::Int64, ::Int64) at ./abstractarray.jl:405
 [6] alignment(::IOContext{Base.TTY}, ::Base.LogicalIndex{Int64,Array{Bool,1}}, ::UnitRange{Int64}, ::UnitRange{Int64}, ::Int64, ::Int64, ::Int64) at ./arrayshow.jl:67
 [7] print_matrix(::IOContext{Base.TTY}, ::Base.LogicalIndex{Int64,Array{Bool,1}}, ::String, ::String, ::String, ::String, ::String, ::String, ::Int64, ::Int64) at ./arrayshow.jl:186
 [8] print_matrix at ./arrayshow.jl:159 [inlined]
 [9] print_array at ./arrayshow.jl:308 [inlined]
 [10] show(::Base.TTY, ::MIME{Symbol("text/plain")}, ::Base.LogicalIndex{Int64,Array{Bool,1}}) at ./arrayshow.jl:345
 [11] show(::Base.TTY, ::String, ::Base.LogicalIndex{Int64,Array{Bool,1}}) at ./multimedia.jl:109
 [12] top-level scope at REPL[7]:1

[karan0299]

I am currently working on this issue.

[karan0299]

@stevengj What is the expected outcome of show(stdout, "text/plain", Base.LogicalIndex([true]))?
According to definition of show function above must output all the elements of array. But since for LogicalIndex the values which are false are ignored while counting the number of elements(or simply false values are ignored as elements) so the output must be the displaying the 'true' number of times it appears in the input array.

[stevengj]

There are many different options for display, of course, but since it is defined as a collection of indices my suggestion would be to display the indices, similar to the output of calling collect on the LogicalIndex.

However, since it doesn't support getindex, I'm skeptical that LogicalIndex should be a subtype of AbstractArray at all. Shouldn't it just be an iterator? (Since this type is not exported or documented, we are free to redefine it, right?)

[stevengj]

cc @mbauman, since he originated this type in #19730.

[mbauman]

FWIW, I intentionally left this unimplemented because I didn't think it'd ever get exposed to the user. I'd be curious what prompted the original report if you're willing to share, @jlumpe.

[jlumpe]

Just ran into it when looking into how the indexing internals work and saw the error when creating an instance in the REPL.

I agree with @stevengj, it seems to me that implementation of getindex is essential for any subtype of AbstractArray.

[mbauman]

Yes, this is a bit of a hack as an internal optimization — and really isn't intended to leak out or be broadly used (hence no nice display). We typically define AbstractArray as having a "fast" scalar lookup; ideally O(1), but hopefully no worse than O(log(n)). Random access into this type is O(n), so I really didn't want to define scalar indexing for it because it'd be a huge boondoggle if you inadvertently tried to use a naive algorithm on it. At the same time, we define our indexing operations to require either scalar indices or AbstractArrays of scalar indices — so that's why it is a subtype of AbstractArray even though it doesn't fully satisfy the AbstractArray interface.