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

define generic in in terms of any #55669

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

Conversation

matthias314
Copy link
Contributor

I think that the current implementation of x in itr is identical to any(==(x), itr), see below. An added bonus of calling any is that a more efficient implementation of any for a custom type will most likely give an efficient implementation of in for that type at the same time.

julia/base/reduce.jl

Lines 1220 to 1238 in 3a2a4d8

any(f, itr) = _any(f, itr, :)
for ItrT = (Tuple,Any)
# define a generic method and a specialized version for `Tuple`,
# whose method bodies are identical, while giving better effects to the later
@eval function _any(f, itr::$ItrT, ::Colon)
$(ItrT === Tuple ? :(@_terminates_locally_meta) : :nothing)
anymissing = false
for x in itr
v = f(x)
if ismissing(v)
anymissing = true
else
v && return true
end
end
return anymissing ? missing : false
end
end

@adienes
Copy link
Contributor

adienes commented Sep 2, 2024

big thumbs up to remove special-cased missing logic !

Copy link
Member

@aviatesk aviatesk left a comment

Choose a reason for hiding this comment

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

LGTM.

@mikmoore
Copy link
Contributor

mikmoore commented Sep 4, 2024

This doesn't pertain to the current PR, which merely matches the existing behavior of using ==.

But I will gripe that in's ambiguity on the subject of == vs isequal is awful:

  in(item, collection) -> Bool
  ∈(item, collection) -> Bool

  Determine whether an item is in the given collection, in the sense that it is == to one of the values generated by
  iterating over the collection. Return a Bool value, except if item is missing or collection contains missing but not
  item, in which case missing is returned (three-valued logic (https://en.wikipedia.org/wiki/Three-valued_logic),
  matching the behavior of any and ==).

  Some collections follow a slightly different definition. For example, Sets check whether the item isequal to one of
  the elements; Dicts look for key=>value pairs, and the key is compared using isequal.

I find the choice of == to be questionable with regard to AbstractFloat behavior

julia> -0.0 in [+0.0] # debatable, but if I wanted `==` or `a <= b <= c` I probably should have written that
true

julia> NaN in [NaN] # these are `===`
false

and think the fact that Set (and others?) has different behavior is abhorrent

julia> -0.0 in Set([+0.0])
false

julia> NaN in Set([NaN])
true

@aviatesk
Copy link
Member

aviatesk commented Sep 6, 2024

Since that behavior is clearly stated in the docstring, I assume it's the result of a lengthy discussion. Rather than debating it in this PR, it seems more appropriate to discuss it in a separate issue.

@aviatesk
Copy link
Member

aviatesk commented Sep 6, 2024

@nanosoldier runbenchmarks(!"scalar", vs=":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

@matthias314
Copy link
Contributor Author

It's hard for me to interpret the benchmark results. Do they show any problem?

@nsajko nsajko added the collections Data structures holding multiple items, e.g. sets label Sep 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
collections Data structures holding multiple items, e.g. sets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants