-
Notifications
You must be signed in to change notification settings - Fork 41
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
Make StructArrays broadcast aware #90
Conversation
Codecov Report
@@ Coverage Diff @@
## master #90 +/- ##
==========================================
- Coverage 91.08% 90.86% -0.23%
==========================================
Files 9 9
Lines 370 372 +2
==========================================
+ Hits 337 338 +1
- Misses 33 34 +1
Continue to review full report at Codecov.
|
Thanks! I'll need @mbauman review as well as I'm not super familiar with the broadcast machinery. The example s = StructArray{ComplexF64}((rand(2,2), rand(2,2)))
s .+ s is very convincing, but what should one expect from s = StructArray{ComplexF64}((rand(2,2), rand(2,2)))
t = rand(2,2)
s .+ t Is the rule that, in order for the result to be a StructArray, all members of the broadcasted operation have to be StructArrays and the resulting eltype a struct? |
|
||
@testset "broadcast" begin | ||
s = StructArray{ComplexF64}((rand(2,2), rand(2,2))) | ||
@test isa(s .+ s, StructArray) |
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.
Probably this also needs to test that things like broadcast(t -> 1, s)
don't give an error (I'm not sure what exactly should happen when the result of broadcasting StructArrays has a eltype that is not a struct).
This won't work for the case where your StructArray contains a different array than |
Could this help with #124? |
Superseded by #136. |
Fixes #89
cc @mbauman who can say whether this is right