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

Add tests for more permissive any/all with non-boolean collections #18969

Merged
merged 1 commit into from
Dec 18, 2016

Conversation

fcard
Copy link
Contributor

@fcard fcard commented Oct 16, 2016

.

@fcard
Copy link
Contributor Author

fcard commented Oct 16, 2016

Ah, found it, the change in any/all behavior was from this commit: 06c93ce
For reference!

@tkelman
Copy link
Contributor

tkelman commented Oct 17, 2016

so map and comprehensions are fine now?

@fcard
Copy link
Contributor Author

fcard commented Oct 17, 2016

They don't short-circuit, otherwise yes.

@tkelman
Copy link
Contributor

tkelman commented Oct 17, 2016

but calling any or all on the result of a map or comprehension will still short-circuit, right?

@fcard
Copy link
Contributor Author

fcard commented Oct 17, 2016

Only if the result has eltype == Bool.

function timeit()
  t_any_some = Any[false; trues(10)]
  t_any_many = Any[false; trues(1000000)]
  t_bool = Bool[false; trues(1000000)]

  res = map(all, (t_any_some, t_any_many, t_bool)) #compile
  @time all(t_any_some)
  @time all(t_any_many)
  @time all(t_bool)
  res
end
julia> timeit()
  0.000001 seconds
  0.027037 seconds
  0.000004 seconds
(false,false,false)

@kshyatt kshyatt added test This change adds or pertains to unit tests error handling Handling of exceptions by Julia or the user labels Oct 17, 2016
@fcard
Copy link
Contributor Author

fcard commented Dec 9, 2016

Because of #19543, this is no longer valid as it is. What do?

  • Re-purpose it to remove the now irrelevant error message?
  • Make this only about the tests?
  • Just close this and discard the changes?
  • Or maybe take the relevant parts there?

@tkelman
Copy link
Contributor

tkelman commented Dec 9, 2016

The tests seem worth keeping

@fcard
Copy link
Contributor Author

fcard commented Dec 10, 2016

Neat, thanks @tkelman!
I will get to work as soon as that's merged (will have to change them a lil' bit to get them working after that)

@fcard fcard force-pushed the fix-non-boolean-indent branch from b055a1a to be4cedb Compare December 17, 2016 20:47
@fcard fcard changed the title Update error message and tests for more permissive any/all with non-boolean collections Add tests for more permissive any/all with non-boolean collections Dec 17, 2016
@fcard
Copy link
Contributor Author

fcard commented Dec 17, 2016

It is done.

Copy link
Contributor

@tkelman tkelman left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @fcard!

@kshyatt kshyatt merged commit 08fcc0f into JuliaLang:master Dec 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error handling Handling of exceptions by Julia or the user test This change adds or pertains to unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants