Skip to content

Reshape accepts a single colon for AbstractArrays #220

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 29, 2021
Merged

Reshape accepts a single colon for AbstractArrays #220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ca63e55
reshape accepts one colon for AbstractArrays
jishnub Mar 8, 2021
8aab6ed
Add tests with different ndims
jishnub Mar 8, 2021
e8740a5
fix offset for colon
jishnub Mar 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/utils.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ _indexlength(i::Integer) = i
_indexlength(i::Colon) = Colon()

_offset(axparent::AbstractUnitRange, ax::AbstractUnitRange) = first(ax) - first(axparent)
_offset(axparent::AbstractUnitRange, ax::Integer) = 1 - first(axparent)
_offset(axparent::AbstractUnitRange, ::Union{Integer, Colon}) = 1 - first(axparent)
Copy link
Member

@johnnychen94 johnnychen94 Apr 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can let OffsetArray constructor decide how to deal with Colon:

Suggested change
_offset(axparent::AbstractUnitRange, ::Union{Integer, Colon}) = 1 - first(axparent)
_offset(axparent::AbstractUnitRange, ::Integer) = 1 - first(axparent)
_offset(axparent::AbstractUnitRange, ::Colon) = Colon()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may do this, but this would mean interpreting Colon as an offset as opposed to an axis as we do now. So after this a Tuple{Vararg{Union{Integer, Colon}}} may be seen as offsets and other combinations (eg. Colons and AbstractUnitRanges, CartesianIndices etc) as axes. I think it should be clear from the context what the meaning of the colon is.


"""
OffsetArrays.AxisConversionStyle(typeof(indices))
Expand Down
21 changes: 21 additions & 0 deletions test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1363,6 +1363,27 @@ end
@test reshape(OffsetArray(-1:0, -1:0), :) == OffsetArray(-1:0, -1:0)
@test reshape(A, :) == reshape(A0, :)

# reshape with one Colon for AbstractArrays
B = reshape(A0, -10:-9, :)
@test B isa OffsetArray{Int,2}
@test parent(B) === A0
@test axes(B, 1) == -10:-9
@test axes(B, 2) == axes(A0, 2)

B = reshape(A0, -10:-9, 3:3, :)
@test B isa OffsetArray{Int,3}
@test same_value(A0, B)
@test axes(B, 1) == -10:-9
@test axes(B, 2) == 3:3
@test axes(B, 3) == 1:2

B = reshape(A0, -10:-9, 3:4, :)
@test B isa OffsetArray{Int,3}
@test same_value(A0, B)
@test axes(B, 1) == -10:-9
@test axes(B, 2) == 3:4
@test axes(B, 3) == 1:1

# pop the parent
B = reshape(A, size(A))
@test B == A0
Expand Down