-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Implement reduce_empty(op, Union{}) #35843
Conversation
@timholy's feedback in JuliaLang/www.julialang.org#794 (comment):
OK maybe saying it's a "bug" is too much. But I think it is a fault of the caller to invoke |
This also fixes a bug mentioned in #28777 (comment) On Julia 1.6-DEV: julia> reduce(*, ())
"" After this PR: julia> reduce(*, ())
ERROR: ArgumentError: reducing over an empty collection is not allowed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Though note that since (as you showed) reduce(*, ()) == ""
currently, making it throw an error may technically break some code.
(cherry picked from commit f705e60)
Interestingly, the invalidation hunt #35733 actually fixed a bug in
reduce_empty
. As #35733 may not be merged as-is, I extracted the bug fix part from #35733 and added a test to it.It requires https://github.com/JuliaLang/Statistics.jl/pull/36