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 18 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
8 changes: 8 additions & 0 deletions src/array/methods.jl
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,14 @@ $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
Base.stack(iter::AbstractArray{<:AbstractDimArray}; dims=:) = Base._stack(dims, check_stack_dims(iter))
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
Base.stack(iter::AbstractArray{<:AbstractDimArray}; dims=:) = Base._stack(dims, check_stack_dims(iter))
Base.stack(iter::AbstractArray{<:AbstractDimArray}; dims=:) = Base._stack(dimnum(first(iter), dims), check_stack_dims(iter))

This should allow passing Dimension/Symbol etc

Copy link
Author

Choose a reason for hiding this comment

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

What about:

Base._stack(dims::Dimension, iter) = Base._stack(typeof(dims), iter)
Base._stack(dims::Type{<:Dimension}, iter) = Base._stack(dimnum(first(iter), dims), iter)

This helps because the default dims in Base's Base.stack is (oddly) dims=:. This has the same effect as dims=ndims(first(iter))+1, as far as I can tell, but it doesn't play well with dimnums in this context.

Copy link
Owner

@rafaqz rafaqz Nov 17, 2024

Choose a reason for hiding this comment

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

Try to avoid dispatch on the underscore as much as possible... And we also need to accept Symbol. I would just define another method like _maybe_dimnum that has dispatch for Colon and Int that don't call dimnum and everything else uses dimnum

Base.stack(f, iter::AbstractArray{<:AbstractDimArray}; dims=:) = Base._stack(dims, f(x) for x in check_stack_dims(iter))
Base.stack(f, xs::AbstractArray{<:AbstractDimArray}, yzs...; dims=:) = _stack(dims, f(xy...) |> check_stack_dims for xy in zip(xs, yzs...))

function Base.inv(A::AbstractDimArray{T,2}) where T
newdata = inv(parent(A))
newdims = reverse(dims(A))
Expand Down
29 changes: 29 additions & 0 deletions test/methods.jl
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,35 @@ 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)...)

x = DimArray([da, ca], (Dim{:a}(1:2),))
sx = stack(x; dims=1)
@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)
end
end

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