Skip to content
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 zero on array of arrays etc apply recursively #51458

Merged
merged 6 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ New library features
--------------------
* `replace(string, pattern...)` now supports an optional `IO` argument to
write the output to a stream rather than returning a string ([#48625]).
* `zero(::AbstractArray)` now applies recursively, so `zero([[1,2],[3,4,5]])` now produces `[[0,0],[0,0,0]]` rather than erroring [#38064].

Standard library changes
------------------------
Expand Down
4 changes: 3 additions & 1 deletion base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1198,7 +1198,8 @@ function copymutable(a::AbstractArray)
end
copymutable(itr) = collect(itr)

zero(x::AbstractArray{T}) where {T} = fill!(similar(x, typeof(zero(T))), zero(T))
zero(x::AbstractArray{T}) where {T<:Number} = fill!(similar(x, typeof(zero(T))), zero(T))
zero(x::AbstractArray) = map(zero, x)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this use broadcasting instead, as custom sparse arrays might define their own broadcast styles and don't need to iterate over the entire array?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe? idk

Custom sparse arrays should probably be implementing their own zero function outright.
Since they likely have their own opinions about whether or not to store structural zeros etc (the SparseArrays stdlib does this).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also a whole lot easier to define a single-argument map(f, ::CustomArray) than it is to define your own broadcast style. I think the map is fine; it's straightforward and obvious and even enables the laser-focused map(::typeof(zero), ::CustomArray).


## iteration support for arrays by iterating over `eachindex` in the array ##
# Allows fast iteration by default for both IndexLinear and IndexCartesian arrays
Expand Down Expand Up @@ -3540,3 +3541,4 @@ function circshift!(a::AbstractVector, shift::Integer)
reverse!(a)
return a
end

oxinabox marked this conversation as resolved.
Show resolved Hide resolved
11 changes: 11 additions & 0 deletions test/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1860,3 +1860,14 @@ f45952(x) = [x;;]
@test_throws "invalid index: true of type Bool" isassigned(A, 1, true)
@test_throws "invalid index: true of type Bool" isassigned(A, true)
end

@testset "zero" begin
@test zero([1 2; 3 4]) isa Matrix{Int}
@test zero([1 2; 3 4]) == [0 0; 0 0]

@test zero([1.0]) isa Vector{Float64}
@test zero([1.0]) == [0.0]

@test zero([[2,2], [3,3,3]]) isa Vector{Vector{Int}}
@test zero([[2,2], [3,3,3]]) == [[0,0], [0, 0, 0]]
end
oxinabox marked this conversation as resolved.
Show resolved Hide resolved
inkydragon marked this conversation as resolved.
Show resolved Hide resolved