findfirst dispatches on ::Function

[jariji]

findfirst(predicate::Function, A)
findfirst(pattern::AbstractString, string::AbstractString)

This function dispatches on Function to avoid ambiguity with the pattern method, so it is not generic to callable types. This seems like it should be two functions: one that takes a predicate and one that takes a pattern.

[KristofferC]

You can just pass x->callable_type(x).

[jariji]

@JeffBezanson wrote

With multiple dispatch, just as the meaning of a function is important to get consistent, it's also important to get the meanings of the argument slots consistent when possible. It's not ideal for a function to treat an argument in a fundamentally different way based on its type

It's not even possible to implement the pattern method in terms of the predicate method, since the pattern matches against slices of the string, not elements. They have quite different contracts.

I propose deprecating the pattern method and introducing another function if desired.

x -> f(x) is a fine workaround but not a good state for the language to be in.

[elextr]

Looks consistent if its considered in the general sense:

findfirst(of_this, in_this)

[inkydragon]

Looks consistent if its considered in the general sense:

This is true in most cases, but there is another do statement for function types:

help?> do
search: do Docs download DomainError @doc Cdouble stdout @__dot__ ReadOnlyMemoryError fieldoffset isreadonly dropdims

  do

  Create an anonymous function and pass it as the first argument to a function call. For example:

  map(1:10) do x
      2x
  end

julia> findfirst(1:10) do x
           x > 5
       end
6

[nalimilan]

See #10593 for the discussions that have led to the current design.

Do we have any actual examples where one would like to pass a callable that isn't a function?

[jariji]

As always in these cases, we need to worry about the situation where a generic package function A.f takes a generic predicate p from outside and calls findfirst(p, x). Currently A.f needs to use findfirst(x->p(x), xs) because they don't control p.

That's awkward enough to warrant avoiding this behavior, but it's worse than that: often A won't think of that and will just use findfirst(p, xs), so its correctness will depend on never receiving an AbstractString predicate. Obviously that isn't a typical type, but in generic programming we are obliged to handle it correctly.

However the motivation for a new function isn't only generic programming, but also human understanding. As mentioned, the two methods have totally different meanings: one matches a substring while the other takes a single element. It's easier to make sense of a system where each name refers to a single concept rather than multiple vaguely related ones.

---

The pattern method has more in common with match(::Regex, s) than its current residence, but accidentally searching for regex as string might be too big a risk to put it there. I propose firstmatch or findsubstring or findslice or similar.

[nsajko]

This function dispatches on Function to avoid ambiguity with the pattern method

This doesn't seem correct: I just tried relaxing Function and Callable parameter annotations for many functions and types in Base and its submodules. Including, for example, findfirst, findlast, findnext, findprev, findall, isapprox. Only one ambiguity appears:

julia> detect_ambiguities(Base, recursive=true)
1-element Vector{Tuple{Method, Method}}:
 (findnext(testf, B::BitArray, start::Integer) @ Base bitarray.jl:1528, findnext(pat::Base.RegexAndMatchData, str, i) @ Base regex.jl:617)

So I guess the type restrictions should just be relaxed from <:Function to <:Any, no breaking changes required?

[nalimilan]

Technically there may be only one ambiguity, but given that there are methods taking AbstractString, AbstractPattern, AbstractChar and AbstractVector{<:Union{Int8,UInt8}} as the first argument, any predicate that would happen to be of these types wouldn't be treated as such. That seems like a relatively unlikely situation, but that doesn't fix issues mentioned in @jariji's last comment, and it makes the API less reliable.