-
Notifications
You must be signed in to change notification settings - Fork 51
improve broadcast_dims to work directly with Dimensions
#775
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
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.
Dimensionsbroadcast_dims to work directly with Dimensions
broadcast_dims to work directly with Dimensionsbroadcast_dims to work directly with Dimensions
rafaqz
left a comment
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?
| 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.