Move StaticArrays support to extension
#265
Conversation
|
Why replace StaticArraysCore with Requires on earlier Julia versions? |
|
Not sure if guys want a hard dependency or a |
Update Project.toml
|
Hey, sorry for the slow reply! So, looks good overall, I've just left a comment to propose to split the changes to broadcast and the refactor into separate PRs (unless there's a strong reason to do them at the same time). EDIT: I now remembered the problem of the extra allocations if the implementation only relies on StaticArraysCore, so I imagine that is the strong reason. Just to make sure I understand, Adapt is still a hard dependency as it doesn't really impact load time? I imagine we could move to weakdeps in the future anyways. |
And use curried adapter to avoid possible instability
|
What's the state on this? |
It's almost done. Most of the suggestions have been resolved. |
| eltys = Tuple{map(eltype, a)...} | ||
| (), Core.Compiler.return_type(f, eltys) | ||
| else | ||
| temp = __broadcast(f, sz, s, a...) |
There was a problem hiding this comment.
This part worries me a little bit, we are using something explicitly marked as internal in StaticArrays. Is there no way to achieve this using only public methods? Or maybe we could check over at StaticArrays if they can offer some solution (maybe add a public method that does what we need).
There was a problem hiding this comment.
Well __broadcast was splitted from _broadcast in JuliaArrays/StaticArrays.jl#1001.
So perhaps the best solution is defining it ourselves.
There was a problem hiding this comment.
Ah I see, thanks for pointing out that discussion. In that case, maybe one could just add a small docstring in StaticArrays.__broadcast to mention that it is the method to be used by outside packages to implement broadcasting of wrapped static arrays (in that way it doesn't accidentally get removed in some StaticArrays refactor that would accidentally break our code).
There was a problem hiding this comment.
That makes sense too.
Anyway, there's not much code added here. Should we just merge this PR as is, and revert that specific commit once StaticArrays get that mention.
|
My thanks as well @N5N3 ! I'm depending on StructArrays more and more, and having it more lightweight is awesome. |
|
Yes, they are independent @piever |
close #263.
Now
StructSizedArray's broadcast won't causes extra allocation.I think we'd better wait for the release of 1.9.