Normalize indices in promote_shape error messages (take 2)

[tpapp]

Seeing implementation like Base.OneTo in error messages may be confusing to some users (cf discussion in #39242,
discourse).

This PR turns

julia> ones(2, 3) + ones(3, 2)
ERROR: DimensionMismatch("dimensions must match: a has dims (Base.OneTo(2), Base.OneTo(3)), b has dims (Base.OneTo(3), Base.OneTo(2)), mismatch at 1")

into

julia> ones(2, 3) + ones(3, 2)
ERROR: DimensionMismatch("dimensions must match: a has axes (1:2, 1:3), b has axes (1:3, 1:2), mismatch at 1")

Fixes #40118.

(This is basically #40124, but redone because I made a mess rebasing).

[mcabbott]

Might be cleaner just to say "a has size (2, 3)" when axes are OneTo?

Also, if changing things, could we kill the repetition in "DimensionMismatch("dimensions must match" ? It looks like a mistake although, when compared to ones(2, 3) .+ ones(3, 2)'s "DimensionMismatch("arrays could not be broadcast to a common size" perhaps it is trying to say that exact agreement is expected here. But surely it could say so without looking like a mistake, maybe "all nontrivial dimensions must agree" (since ones(2,3) + ones(2,3,1) is fine)

[tpapp]

Might be cleaner just to say "a has size (2, 3)" when axes are OneTo?

That is indeed shorter, but at the same time I think the language should encourage people to think in terms of generalized indexing all the time since it is part of the <:AbstractArray interface, and error messages should be part of that. I think that the extra 1:s are a small price for that, and consistency.

kill the repetition

done, thanks.

[mcabbott]

extra 1:s are a small price

My concern isn't brevity, it's more that zeros(1:2, 1:3) isa OffsetArray while zeros(2, 3) isa Array -- OneTo(3) has special printing precisely because it's not equivalent to 1:3.

[tpapp]

I think the error message is pretty clear about these axes; I am not quite sure how zeros comes into it.

Also, what should happen for two arrays with axes eg (Base.OneTo(4),) and (3:5, )?

[mcabbott]

My point is that, for axes, 1:4 and OneTo(4) are meaningfully different. So I think we should be a little cautious about printing one as the other, which so far Base avoids doing. They are == of course, and the error is !=, so it's not crazy... but it strays from what Base would accept as input. (zeros is just an example, reshape etc. behave the same way.)

What should be printed in cases where some but not all of the axes in play are Base.OneTo, that's less obvious. If you're complaining that mixing size & axes in one message would be odd, then I agree, we shouldn't do that. This is certainly quite a mouthful at present:

julia> reshape(rand(3), 2:4) + ones(3)
ERROR: DimensionMismatch("dimensions must match: a has dims (OffsetArrays.IdOffsetRange(values=2:4, indices=2:4),), b has dims (Base.OneTo(3),), mismatch at 1")

Maybe printing something like axes(a) == (2:4,) but axes(b) == (1:3,) might make sense in the presence of offsets. To stress that this isn't printed in re-digestible form like repr, only up to ==.

[tpapp]

are meaningfully different. [...] They are == of course

Sorry, I don't understand how values that are == can be considered meaningfully different.

I think that Base.OneTo is an irrelevant implementation detail that should be hidden from the user to the extent possible (cf the discussion in #39242). This is what this PR aims to do, nothing more.

[mcabbott]

Sorry, I don't understand how values that are == can be considered meaningfully different.

I thought I gave lots of examples. One more, on a fresh session:

julia> reshape(rand(4), Base.OneTo(2), Base.OneTo(2)) |> summary
"2×2 Matrix{Float64}"

julia> reshape(rand(4), 1:2, 1:2)
ERROR: MethodError: no method matching reshape(::Vector{Float64}, ::Tuple{UnitRange{Int64}, UnitRange{Int64}})

[antoine-levitt]

Ping, this is still grating and would be great to fix. Reading up the discussion, it seems that @mcabbott 's objection is that we need to carefully distinguish different types of axes, but it seems to me that it would be OK to have a simpler (possibly different) error message in the common cases of "standard" axes. @mcabbott do you think that's acceptable? If yes, any thoughts as to how to proceed?

[tpapp]

@antoine-levitt: if triage gives a clear direction, I would be happy to update this PR.

FWIW, I would leave it as is: the only documented methods of reshape in Base are those that deal with dims::Tuple{Vararg{Union{Colon, Int64}}}. When (if) this part of the API is made more consistent, we can address that in another PR.

[vtjnash]

I have tried to take the feedback of the discussion into account, so now it reads, and I think should be ready to merge:

julia> ones(2, 3) + ones(2, 4)
ERROR: DimensionMismatch: a has size (2, 3), b has size (2, 4), mismatch at dim 2
Stacktrace:
 [1] throw_promote_shape_mismatch(a::Tuple{Base.OneTo{Int64}, Base.OneTo{Int64}}, b::Tuple{Base.OneTo{Int64}, Base.OneTo{Int64}}, i::Int64)
   @ Base ./indices.jl:135
 [2] promote_shape
   @ ./indices.jl:196 [inlined]
 [3] promote_shape
   @ ./indices.jl:188 [inlined]
 [4] +(A::Matrix{Float64}, Bs::Matrix{Float64})
   @ Base ./arraymath.jl:14
 [5] top-level scope
   @ REPL[1]:1

julia> include("test/testhelpers/OffsetArrays.jl")
Main.OffsetArrays

julia> reshape(rand(3), 2:4) + ones(3)
ERROR: DimensionMismatch: a has axes (2:4,), b has axes (1:3,), mismatch at dim 1
Stacktrace:
 [1] throw_promote_shape_mismatch(a::Tuple{Main.OffsetArrays.IdOffsetRange{Int64, Base.OneTo{Int64}}}, b::Tuple{Base.OneTo{Int64}}, i::Int64)
   @ Base ./indices.jl:135
 [2] promote_shape
   @ ./indices.jl:196 [inlined]
 [3] promote_shape
   @ ./indices.jl:188 [inlined]
 [4] +(A::Main.OffsetArrays.OffsetVector{Float64, Vector{Float64}}, B::Vector{Float64})
   @ Base ./arraymath.jl:7
 [5] top-level scope
   @ REPL[3]:1