Default `reverse`

[nchisholm]

Applying reverse to a DimArray (without a dims= argument) reverses along the first dimension only. On the other hand, a regular Array has all its dimensions reversed. Should the default behavior of reverse(::DimArray) be changed to act along all dimensions to be more consistent with the usual behavior? If this was instead an intentional design choice, perhaps we could add a note to the documentation somewhere.

Example:

julia> da = DimArray{Int}(undef, X(1:5), Y(1:4));

julia> da[:] = 1:20
1:20

julia> da
╭───────────────────────╮
│ 5×4 DimArray{Int64,2} │
├───────────────────────┴────────────────────────── dims ┐
  ↓ X Sampled{Int64} 1:5 ForwardOrdered Regular Points,
  → Y Sampled{Int64} 1:4 ForwardOrdered Regular Points
└────────────────────────────────────────────────────────┘
 ↓ →  1   2   3   4
 1    1   6  11  16
 2    2   7  12  17
 3    3   8  13  18
 4    4   9  14  19
 5    5  10  15  20

julia> reverse(da)  # acts on only the first (X) dimension
╭───────────────────────╮
│ 5×4 DimArray{Int64,2} │
├───────────────────────┴───────────────────────────── dims ┐
  ↓ X Sampled{Int64} 5:-1:1 ReverseOrdered Regular Points,
  → Y Sampled{Int64} 1:4 ForwardOrdered Regular Points
└───────────────────────────────────────────────────────────┘
 ↓ →  1   2   3   4
 5    5  10  15  20
 4    4   9  14  19
 3    3   8  13  18
 2    2   7  12  17
 1    1   6  11  16

julia> reverse(parent(da))  # acts on all dimensions; equivalent to reverse(parent(da), dims=:)
5×4 Matrix{Int64}:
 20  15  10  5
 19  14   9  4
 18  13   8  3
 17  12   7  2
 16  11   6  1

Also, note that reverse(da, dims=:) currently throws "ArgumentError: dims are not Dimensions", although it works (and is the default) for Arrays. Perhaps we could support it to give an easy way to reverse along all dimensions, especially if defaulting to that behavior is problematic.

[rafaqz]

Thanks for filing this, I guess I never use reverse on multidimensional arrays without dims. It should totally be fixed.

As a rule anything like that this isn't exactly the same as Base AbstractArray behavior is a bug rather than something intentional. A DimArray should be a conforming AbstractArray except in cases where AbstractArray would error - e.g. by using a Dimension in dims for `reverse. (bugs like this exist because we add methods to try and make the dimensions do sensible things to match the array behaviour, and sometimes don't get the implementation right)

I will try to enforce this when the BaseInterfaces.jl array interface is more complete, its just hard to comprehensively test every part of the AbstractArray interface currently.

[nchisholm]

You're welcome, and seems like a quick fix. May I submit a PR?

[rafaqz]

Absolutely!