Skip to content

Make zero on array of arrays etc apply recursively #51458

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

Merged
merged 6 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -109,6 +109,7 @@ New library features
automatically.
* `@timed` now additionally returns the elapsed compilation and recompilation time ([#52889])
* `filter` can now act on a `NamedTuple` ([#50795]).
* `zero(::AbstractArray)` now applies recursively, so `zero([[1,2],[3,4,5]])` now produces the additive identity `[[0,0],[0,0,0]]` rather than erroring ([#38064]).

Standard library changes
------------------------
Expand Down
3 changes: 2 additions & 1 deletion base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1217,7 +1217,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
Member

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
11 changes: 11 additions & 0 deletions test/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1950,3 +1950,14 @@ end
@test repeat(f, 2, 3, 4) === FillArrays.Fill(3, (8, 6, 4))
@test repeat(f, inner=(1,2), outer=(3,1)) === FillArrays.Fill(3, (12, 4))
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