Skip to content
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.

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

Extending Base.stack for DimArrays #645

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
4ea5e40
Extend Base.stack to DimArrays
brendanjohnharris Feb 23, 2024
ec3b2cf
Initial Base.stack tests
brendanjohnharris Feb 23, 2024
0b762ce
Fix whitespace changes
brendanjohnharris Feb 23, 2024
6affd9b
Merge branch 'stack' into stacker
brendanjohnharris Feb 23, 2024
ac3affc
Clean up Base.stack, add docstrings, add tests
brendanjohnharris Feb 27, 2024
83ace60
Fix Base.stack docstring
brendanjohnharris Feb 27, 2024
e6cfb1b
New Pair syntax for extended Base.stack
brendanjohnharris Mar 1, 2024
ce16068
Better Base.stack formatting
brendanjohnharris Mar 7, 2024
bdf151f
Tweak docstring for Base.stack, remove kwargs
brendanjohnharris Mar 7, 2024
7a4cf26
Merge branch 'rafaqz:main' into stack
brendanjohnharris Mar 7, 2024
3314852
Update whitespace
brendanjohnharris Mar 7, 2024
0ac2eb0
Fix whitespace
brendanjohnharris Mar 7, 2024
884ab68
Fix whitespace
brendanjohnharris Mar 7, 2024
c369d06
use rebuild
rafaqz May 19, 2024
2c7b8b9
rebuild
rafaqz May 19, 2024
cf57f86
Merge branch 'rafaqz:main' into stack
brendanjohnharris Nov 15, 2024
0a278a6
Update Base.stack tests
brendanjohnharris Nov 15, 2024
f41f424
Rework Base.stack
brendanjohnharris Nov 15, 2024
d9ee5e7
Update Base.stack to accept dims<:Dimension
brendanjohnharris Nov 17, 2024
9b3b3b7
Improve Base.stack tests
brendanjohnharris Nov 17, 2024
25dc8a2
Add test for Base.stack with symbol dims
brendanjohnharris Nov 17, 2024
e2aeb0e
Replace accidentally deleted line for Base.stack
brendanjohnharris Nov 17, 2024
39c78e8
Update Base.stack for symbol dims
brendanjohnharris Nov 17, 2024
16c205a
Fix typo
brendanjohnharris Nov 17, 2024
15cc9fd
Avoid dispatching on Base._stack
brendanjohnharris Nov 19, 2024
52ce34a
Update tests
brendanjohnharris Nov 19, 2024
f868ddf
Update test with error on out-of-range dims
brendanjohnharris Nov 19, 2024
911acda
Update _maybe_dimnum to error if specified Integer dims are too large
brendanjohnharris Nov 19, 2024
9517d16
Use map instead of dot broadcast
brendanjohnharris Nov 21, 2024
b329519
Update reference.md
brendanjohnharris Nov 21, 2024
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
1 change: 1 addition & 0 deletions docs/src/api/reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ Base methods

```@docs
Base.cat
Base.stack
Base.copy!
Base.eachslice
```
Expand Down
47 changes: 47 additions & 0 deletions src/array/methods.jl
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,53 @@ $message on dimension $D.
To fix for `AbstractDimArray`, pass new lookup values as `cat(As...; dims=$D(newlookupvals))` keyword or `dims=$D()` for empty `NoLookup`.
"""

function check_stack_dims(iter)
comparedims(Bool, dims.(iter)...; order=true, val=true, msg=Dimensions.Warn(" Can't `stack` AbstractDimArray, applying to `parent` object."))
iter
end

_maybe_dimnum(x, dim) = hasdim(x, dim) ? dimnum(x, dim) : ndims(x) + 1
function _maybe_dimnum(x, dim::Integer)
if dim < ndims(x) + 2
return dim
else
throw(ArgumentError(LazyString("cannot stack slices ndims(x) = ", ndims(x) + 1, " along dims = ", dim)))
end
end
_maybe_dimnum(_, ::Colon) = Colon()
_maybe_dimnum(x, dims::Tuple) = map(d -> _maybe_dimnum(x, d), dims)

"""
stack(x::AbstractDimArray; [dims])
brendanjohnharris marked this conversation as resolved.
Show resolved Hide resolved

Combine a nested `AbstractDimArray` into a single array by arranging the inner arrays
along one or more new dimensions taken from the outer array.
The inner `AbstractDimArray`s must all have the same dimensions.

The keyword `dims` specifies where the new dimensions will be placed in the resulting array.
The index of all existing dimensions equal to or greater than `dims` will be increased to
accomodate the new dimensions.
The default `dims` places the new dimensions at the end of the resulting array.

# Examples
```julia
using DimensionalData
a = DimArray([1 2 3; 4 5 6], (X(4.0:5.0), Y(6.0:8.0)))
b = DimArray([7 8 9; 10 11 12], (X(4.0:5.0), Y(6.0:8.0)))
x = DimArray([a, b], Z(4.0:5.0)) # Construct a nested DimArray
y = stack(x; dims=2) # Resulting array has dims (X, *Z*, Y)
```
"""
function Base.stack(iter::AbstractArray{<:AbstractDimArray}; dims=:)
Base._stack(_maybe_dimnum(first(iter), dims), check_stack_dims(iter))
end
function Base.stack(f, iter::AbstractArray{<:AbstractDimArray}; kwargs...)
Base.stack(f(x) for x in check_stack_dims(iter); kwargs...)
end
function Base.stack(f, xs::AbstractArray{<:AbstractDimArray}, yzs...; kwargs...)
Base.stack(f(xy...) for xy in zip(xs, yzs...); kwargs...)
end

function Base.inv(A::AbstractDimArray{T,2}) where T
newdata = inv(parent(A))
newdims = reverse(dims(A))
Expand Down
41 changes: 41 additions & 0 deletions test/methods.jl
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,47 @@ end
end
end

@testset "Base.stack" begin
a = [1 2 3; 4 5 6]
da = DimArray(a, (X(4.0:5.0), Y(6.0:8.0)))
b = [7 8 9; 10 11 12]
ca = DimArray(b, (X(4.0:5.0), Y(6.0:8.0)))
db = DimArray(b, (X(6.0:7.0), Y(6.0:8.0)))

x = DimArray([da, db], (Z(4.0:5.0)))
@test_warn "Lookup values" stack(x)

x = DimArray(fill(da, 2, 2), (Dim{:a}(4.0:5.0), Dim{:b}(6.0:7.0)))
sx = stack(x)
@test sx isa AbstractDimArray
@test dims(sx) == (dims(first(x))..., dims(x)...)

@test_throws "ArgumentError" stack(x; dims=4)

x = DimArray([da, ca], (Dim{:a}(1:2),))
sx = stack(x; dims=1)
sy = @test_nowarn stack(x; dims=:)
sz = @test_nowarn stack(x; dims=X)
Copy link
Owner

Choose a reason for hiding this comment

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

What happens with dims=Z ?

Copy link
Author

Choose a reason for hiding this comment

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

Currently, dims=Z (where Z is not a dimension in the DimArrays being stacked) falls back to adding the new dimension at the end (so the result is the same as for the default behavior):

_maybe_dimnum(x, dim) = hasdim(x, dim) ? dimnum(x, dim) : ndims(x) + 1

Should we throw an error instead, like when an out-of-range Integer dim is give? I.e.:

function _maybe_dimnum(x, dim::Int)
    if dim < ndims(x) + 2
        return dim
    else
        throw(ArgumentError(LazyString("cannot stack slices ndims(x) = ", ndims(x) + 1, " along dims = ", dim)))
    end
end

Copy link
Owner

Choose a reason for hiding this comment

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

I was thinking we should add a dimension like it is currently, but we can set it to be a Z dimension

Copy link
Author

@brendanjohnharris brendanjohnharris Dec 9, 2024

Choose a reason for hiding this comment

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

Using the current minimal approach, stacking with a new dimension Z would be:

a = [1 2 3; 4 5 6]
da = DimArray(a, (X(4.0:5.0), Y(6.0:8.0)))
b = [7 8 9; 10 11 12]
db = DimArray(b, (X(4.0:5.0), Y(6.0:8.0)))
x = DimArray([da, db], (Dim{:a}(1:2),))

stack(set(x, :a=>Z)) # Or
set(stack(x), :a=>Z})

We could have this behavior be automatic (without overloading Base._stack) with:

function Base.stack(iter::AbstractArray{<:AbstractDimArray}; dims=:)
    x = Base._stack(_maybe_dimnum(first(iter), dims), check_stack_dims(iter))
    if !hasdim(x, dims) && Z isa Union{Dimension,Type{<:Dimension}}
        x = set(x, DimensionalData.dims(x)[end] => dims)
    end
    return x
end

But I wonder if it could be confusing to have this different behavior for when dims <: Dimension and hasdim(x, dims) (the new dimension is inserted before dims but keeps the original name) versus !hasdims(x, dims) (the new dimension is inserted at the end and renamed to dims); it also means the return type can't be inferred

Copy link
Owner

@rafaqz rafaqz Dec 10, 2024

Choose a reason for hiding this comment

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

Currently, dims=Z (where Z is not a dimension in the DimArrays being stacked) falls back to adding the new dimension at the end (so the result is the same as for the default behavior)

I'm not sure we are understanding each other... My only ask here was that this new dimension is a Z dimension if dims=Z was used

@test sx == sz
@test sx == stack([parent(da), parent(ca)], dims=1)
@test sx isa AbstractDimArray
@test dims(sx) == (dims(x)..., dims(first(x))...)

for d = 1:3
x = DimArray([da, ca], (AnonDim(),))
dc = stack(x, dims=d)
@test dims(dc, d) isa AnonDim
@test parent(dc) == stack([da, db], dims=d)

dc = stack(x -> x .^ 2, x; dims=d)
@test dims(dc, d) isa AnonDim
@test dc == stack(parent(x); dims=d) .^ 2
dc = stack(+, x, x, x; dims=d)
@test dims(dc, d) isa AnonDim
@test dc == stack(x + x + x; dims=d)
end
end

@testset "unique" begin
a = [1 1 6; 1 1 6]
xs = (X, X(), :X)
Expand Down
Loading