-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
narrow array conversions #26330
narrow array conversions #26330
Conversation
Wow, that was easier than I expected. I assume that this then hits the convert deprecation for |
base/array.jl
Outdated
convert(::Type{T}, a::T) where {T<:Array} = a | ||
convert(::Type{T}, a::AbstractArray) where {T<:Array} = T(a) | ||
|
||
convert(::Type{AbstractArray{T}}, a::AbstractArray) where {T} = AbstractArray{T}(a) |
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.
Did you try removing this one, too?
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.
Ah, nevermind. I was getting confused by Type{<:AbstractArray}
and Type{AbstractArray}
. This looks great. For anyone else getting into the same confusion, this dispatches to the AbstractArray{T}(…)
constructor, which definitely exists and just copyto!
s into a similar
array.
From this I've discovered that it's possible to |
49f6f7c
to
765ec85
Compare
Holy cow, this seems to have a huge effect on sysimg build time and size --- basecompiler.ji goes from ~19MB to ~80MB. |
Ok, I thought this caused it but now I've seen the same behavior on master. Appears to be non-deterministic? |
Non-deterministic compiled code size has been reported a few times before. |
(#25900) |
I'm tentatively in support of having |
OK, I agree that might be fine. I'll keep these methods for now then. |
Probably worth marking #23821 for triage? It doesn't make much sense to keep these |
I'm finding the structured matrix constructors a bit hard to navigate. It's not clear when they (should?) check to see if their argument is representable. For example
This might be partly my fault from the constructor-to-convert fallback removal. But I'm guessing the intent is to have all such operations accessible as |
#23821 should be non-breaking. |
765ec85
to
e8fcb98
Compare
Yes, it's non-breaking, but if we eventually decide not to do it, we won't be able to remove |
e8fcb98
to
f977faa
Compare
Not all array types can convert from any AbstractArray via a 1-argument constructor call. Structured array types like
Diagonal
have different constructor behaviors, and some types likeReshapedArray
don't have any such constructors.fixes #26294, fixes #26178