-
-
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
Provide method for sum
that accepts a neutral element
#18423
Comments
What's wrong with calling it |
There's a discussion elsewhere where people complain about / are puzzled about / don't like that Otherwise, we need to update the documentation of |
We could potentially handle this the same way we handle inferring the element type of empty comprehensions. Of course, that approach has its own predictability issues, but in the future, if we decide that the approach is acceptable we could handle empty reductions the same way – i.e. by calling |
@StefanKarpinski, we already call I'm inclined to think this is not a practical problem. If you are summing an
In 95% of such cases, you should probably be using a more concretely typed array. In the remaining 5% of cases, I don't see the problem with calling Adding an extra optional argument to |
See also #5311 |
See #18367 for some relevant discussion as well |
It's possible to just call
There is no precedent for "if this doesn't hold in full generality, bail out". Imagine if julia> Base.vect(1, 2)
2-element Array{Int64,1}:
1
2
julia> Base.vect(1.0, 2.0)
2-element Array{Float64,1}:
1.0
2.0
julia> Base.vect()
ERROR: Cannot figure out type. That's ridiculous, so Same goes for julia> map(x -> 0, 1)
0
julia> map(x -> 0, [1])
1-element Array{Int64,1}:
0
julia> map(() -> 0)
0 There's no correct return type here either. But a scalar is the sane choice, because it will work properly in most cases. The remaining 5% of cases isn't worth throwing an error for. Similarly, I think @stevengj I disagree that it's not a practical problem; it's shown up in |
Because the empty sum has a well defined answer — the additive identity — but there is no single additive identity that works for any type defining In contrast, if Similarly, |
I disagree that |
This issue has been marked with the doc tag. If we aren't going to fix |
The definition for The documentation should definitely suggest However, the type doesn't have to be concrete. It would be perfectly reasonable to define |
(Currently, |
That sounds like a fair solution to me. |
To follow up on the previous comment, I'm still open to changing Unitful, ideally before I register it as a package and people start relying on it to behave a certain way. I had a productive discussion with @timholy about related issues here: PainterQubits/Unitful.jl#9. To avoid derailing this thread, I'm happy to engage you there if you have thoughts specific to whether or not physical quantities should be a subtype of |
Since I've been mentioned here, I'll respond to this (highlighted by me) comment by @stevengj by saying that there seems to be an underlying demand for having access to an abstract additive identity. The fact that there isn't one suggests that we should really consider adding one (or just use the To me, the downside is there is no limit to where the abstractions end. E.g. we could make assumptions like |
Going back to the feasibility of the opening suggestion: I would love it, because it is being grok-able, type-stable and work for empties, but there are practical problems to making the nicest form of this syntax available. For me, a proposal for
So if the Looking at #5311, I realised that the notation |
Currently, the expression
sum([])
leads to an error, since there is no way to know the intended neutral element ("zero") for an array of typeArray{Any}
. I suggest to add a method tosum
(or to provide a new functionsum0
) that accepts such a zero element. This would correspond to the existing methodreduce(op, v0, itr)
.The implementation would be straightforward:
Equivalent methods should be added for
prod
,minimum
, andmaximum
as well.Whether we can use the name
sum
for this, or need to use a new namesum0
, depends on what methods currently exist forsum
and whether there would be a conflict.The text was updated successfully, but these errors were encountered: