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.

Sign up for GitHub

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

Jump to bottom

Add Iterators.first and Iterators.last which don't throw for empty collections #37119

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add Iterators.first and Iterators.last which don't throw for empty collections #37119

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
39a39e9
Add methods of first and last accepting a predicate
non-Jedi Aug 19, 2020
c89a724
Use return keyword instead of implicit return
non-Jedi Aug 19, 2020
f477520
Fixup code style in docstrings
non-Jedi Aug 20, 2020
8c425f7
Add compat annotations to docstrings
non-Jedi Aug 20, 2020
16a19f9
Create Iterators.first and Iterators.last
non-Jedi Aug 27, 2020
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
67 changes: 67 additions & 0 deletions base/abstractarray.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,35 @@ function first(itr)
x[1]
end

"""
first(predicate, coll)

Get the first element of `coll` satisfying `predicate` wrapped in `Some`.
non-Jedi marked this conversation as resolved.
Show resolved Hide resolved

If no element of `coll` satisfies `predicate`, return `nothing`.
Copy link
Member

@mbauman mbauman Aug 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This is interesting — and deviates from the existing first which would throw an error. This gives me a bit of pause, but at a minimum it means that we cannot combine these docstrings:

julia> first(_->true, [])

julia> first([])
ERROR: BoundsError: attempt to access 0-element Array{Any,1} at index [1]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point on the docstrings.


non-Jedi marked this conversation as resolved.
Show resolved Hide resolved
# Examples
```jldoctest
julia> first(>(5), 1:10)
Some(6)

julia> first(isodd, 2:2:10) |> isnothing
non-Jedi marked this conversation as resolved.
Show resolved Hide resolved
true

julia> first(iseven, [5; 3; 4; 2; 6; 8]) |> something
non-Jedi marked this conversation as resolved.
Show resolved Hide resolved
4

julia> something(first(>(10), 1:10), 0)
0
```
"""
function first(predicate, itr)
for x in itr
predicate(x) && return Some(x)
Copy link
Member

Choose a reason for hiding this comment

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

I'm not thrilled about Some here — I don't think we have a single API in base that returns Some. That said, this is unlike the other find/search APIs in that they return indices or RegexMatches or values from some other limited type set that does not include Nothing.

Copy link
Contributor Author

@non-Jedi non-Jedi Aug 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I think I've convinced myself that at the very least this should be a separate function from first/last. Does Iterators.first (as suggested in OP) sound like a good name to you? In that case, we wouldn't have to create a predicate accepting version as it would compose with Iterators.filter. EDIT: this PR now implements Iterators.first instead.

In reality, I think all APIs that generically return an element of a collection but throw if the element isn't present should instead return Union{Some{T},Nothing}. This includes first, last, getindex, reduce, foldl, etc. This is obviously breaking; I should probably create a "Taking something seriously" issue to get feedback on that idea for julia 2.0.

end
non-Jedi marked this conversation as resolved.
Show resolved Hide resolved
return nothing
end

"""
first(itr, n::Integer)

Expand Down Expand Up @@ -388,6 +417,44 @@ julia> last([1; 2; 3; 4])
"""
last(a) = a[end]

"""
last(predicate, coll)

Get the last element of `coll` satisfying `predicate` wrapped in `Some`.
non-Jedi marked this conversation as resolved.
Show resolved Hide resolved

If no element of `coll` satisfies `predicate`, return `nothing`.

non-Jedi marked this conversation as resolved.
Show resolved Hide resolved
# Examples
```jldoctest
julia> last(<(5), 1:10)
Some(4)

julia> last(isodd, 2:2:10) |> isnothing
non-Jedi marked this conversation as resolved.
Show resolved Hide resolved
true

julia> last(iseven, [5; 3; 4; 2; 6; 9]) |> something
non-Jedi marked this conversation as resolved.
Show resolved Hide resolved
6

julia> something(last(>(10), 1:10), 0)
0
```
"""
function last(predicate, itr)
out = nothing
for x in itr
non-Jedi marked this conversation as resolved.
Show resolved Hide resolved
out = ifelse(predicate(x), Some(x), out)
end
return out
end

# faster version for arrays
function last(predicate, a::AbstractArray)
@inbounds for i in reverse(eachindex(a))
predicate(a[i]) && return Some(a[i])
end
non-Jedi marked this conversation as resolved.
Show resolved Hide resolved
return nothing
end

"""
last(itr, n::Integer)

Expand Down
10 changes: 10 additions & 0 deletions test/abstractarray.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1141,3 +1141,13 @@ end
@test last(itr, 1) == [itr[end]]
@test_throws ArgumentError last(itr, -6)
end

@testset "first/last element satisfying predicate of $(typeof(itr))" for itr in (1:9,
collect(1:9),
reshape(1:9, (3, 3)),
ntuple(identity, 9))
@test first(>(5), itr) |> something == 6
@test last(<(5), itr) |> something == 4
@test first(>(9), itr) == nothing
@test last(>(9), itr) == nothing
end