-
Notifications
You must be signed in to change notification settings - Fork 43
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
improve broadcast_dims
to work directly with Dimension
s
#775
base: main
Are you sure you want to change the base?
Conversation
…on`s and `DimArray`s The `broadcast_dims` function now supports broadcasting over `Dimension`s and references to them, in addition to `AbstractDimArray`s. This allows for more flexibility when broadcasting over combinations of `DimArray`s and `Dimension`s.
Dimension
sbroadcast_dims
to work directly with Dimension
s
broadcast_dims
to work directly with Dimension
sbroadcast_dims
to work directly with Dimension
s
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it also work with DimStack?
""" | ||
function broadcast_dims(f, As::AbstractBasicDimArray...) | ||
dims = combinedims(As...) | ||
T = Base.Broadcast.combine_eltypes(f, As) | ||
broadcast_dims!(f, similar(first(As), T, dims), As...) | ||
end | ||
|
||
function broadcast_dims(f, As::Union{AbstractBasicDimArray, Dimensions.Dimension, Type{<:Dimension}, Symbol}...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function broadcast_dims(f, As::Union{AbstractBasicDimArray, Dimensions.Dimension, Type{<:Dimension}, Symbol}...) | |
function broadcast_dims(f, As::Union{AbstractBasicDimArray,Dimension,Type{<:Dimension},Symbol}...) |
I think so yes -- I'll try to look at this again next week. |
Is it worth finishing this? Would still be nice to have consistency with |
Thanks for pinging me again. Are there use cases for keeping both? An alternative would be to deprecate |
I think there are still contexts to use a function, like we still have Right now especially one of those is on |
Closes #748.