Skip to content

Commit 981d26e

Browse files
authored
fix and test broadcast promotedims (#918)
1 parent 5bc2d60 commit 981d26e

File tree

3 files changed

+32
-14
lines changed

3 files changed

+32
-14
lines changed

src/Lookups/lookup_arrays.jl

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -852,21 +852,23 @@ promote_first(x1, x2, xs...) =
852852
# Fallback NoLookup if not identical type
853853
promote_first(l1::Lookup) = l1
854854
promote_first(l1::L, ls::L...) where L<:Lookup = rebuild(l1; metadata=NoMetadata)
855-
function promote_first(l1::L, ls::Lookup...) where {L<:Lookup}
856-
ls = _remove(Length1NoLookup, l1, ls...)
857-
if length(ls) > 1
858-
l1, ls... = ls
859-
else
855+
function promote_first(l1::Lookup, ls1::Lookup...)
856+
ls = _remove(Length1NoLookup, l1, ls1...)
857+
if length(ls) != length(ls1) + 1
858+
# If anything was removed, start again
859+
return promote_first(ls...)
860+
elseif length(ls) == 1
861+
# If there is only one left, use it
860862
return first(ls)
861863
end
862-
if all(map(l -> typeof(l) == L, ls))
863-
if length(ls) > 0
864-
rebuild(l1; metadata=NoMetadata())
865-
else
866-
l1 # Keep metadata if there is only one lookup
867-
end
864+
# Otherwise see if these have the same type
865+
l2, ls2... = ls
866+
if all(map(l -> typeof(l) == typeof(l2), ls2))
867+
# If so, just simplify the metadata
868+
rebuild(l2; metadata=NoMetadata())
868869
else
869-
NoLookup(Base.OneTo(length(l1)))
870+
# And if not, use NoLookup
871+
NoLookup(Base.OneTo(length(l2)))
870872
end
871873
end
872874
# Categorical lookups

src/array/broadcast.jl

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,9 @@ macro d(expr::Expr, options::Union{Expr,Nothing}=nothing)
174174
dims = $DimensionalData.dims(found_dims, order_dims)
175175
end
176176
else
177-
:(dims = _find_dims(vars))
177+
quote
178+
dims = _find_dims(vars)
179+
end
178180
end
179181
quote
180182
let
@@ -404,7 +406,6 @@ _broadcasted_dims(a) = ()
404406
# its dimensions to match the rest of the @d broadcast, otherwise do nothing.
405407
_maybe_dimensional_broadcast(x, _, _) = x
406408
function _maybe_dimensional_broadcast(A::AbstractBasicDimArray, dest_dims, options)
407-
len1s = basedims(otherdims(dest_dims, dims(A)))
408409
# Reshape first to avoid a ReshapedArray wrapper if possible
409410
A1 = _maybe_insert_length_one_dims(A, dest_dims)
410411
# Then permute and reorder

test/broadcast.jl

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,21 @@ end
371371
p(da1, da2, permutedims(da3, (X, Y, Z)))
372372
end
373373

374+
@testset "Lookups are maintained" begin
375+
x = format(X(-5.0:5.0))
376+
y = format(Y(-10.0:2:12.0))
377+
z = format(Z(-3.0:0.5:4.0))
378+
379+
u = @d x .* y
380+
v = @d x .* z
381+
w = @d y .* z
382+
383+
f(u, v, w) = u + v + w
384+
385+
A = @d f.(u, v, w)
386+
@test dims(A) == (x, y, z)
387+
end
388+
374389
@testset "strict" begin
375390
@test_nowarn @d rand(X(1:3)) .* rand(X([:a, :b, :c])) strict=false
376391
@test_throws DimensionMismatch @d rand(X(1:3)) .* rand(X([:a, :b, :c])) strict=true

0 commit comments

Comments
 (0)