JuliaStats / Statistics.jl

The Statistics stdlib that ships with Julia.
https://juliastats.org/Statistics.jl/dev/
Other
71 stars 40 forks source link

Restrict type of f in `mean(f, itr)` #143

Closed KronosTheLate closed 1 year ago

KronosTheLate commented 1 year ago

A naive user (like me) might expect there to be a vararg method for mean, which computes the mean of all arguments if they are numbers [1]. However, if one tries to do that, a not-so-helpful error is produced:

julia> mean(1, 1)
ERROR: MethodError: objects of type Int64 are not callable
Maybe you forgot to use an operator such as *, ^, %, / etc. ?
Stacktrace:
 [1] mean(f::Int64, itr::Int64)
   @ Statistics ~/.julia/juliaup/julia-1.9.0-rc2+0.x64.linux.gnu/share/julia/stdlib/v1.9/Statistics/src/Statistics.jl:69
 [2] top-level scope
   @ REPL[9]:1

This PR restricts the type of f, so that an appropriate MethodError should be thrown (I have not tested).

[1] An alternative fix would be to define such a method, as in mean(args::Number...) = mean([args...]). I am not sure what is preferable, but the suggested PR seems more clear.

nalimilan commented 1 year ago

Unfortunately we can't do that, as we need to allow any callable object, which includes any type. The same problem applies to many methods in Base.

nalimilan commented 1 year ago

Also note that sum(1, 1) has the same problem. This should be changed in Base first if we wanted to change this here.

KronosTheLate commented 1 year ago

So... should this be changed in base? It is an issue here until it is fixed there, so instead of closing this issue, would it not be more appropriate to open an issue in base and refer to this (still existing and therefore open) issue? Or are you saying that this can not really ever be fixed, and is therefore not to be considered an issue?

nalimilan commented 1 year ago

I don't think this can be fixed, so you could try to discuss this in Base but I'm pretty sure about the outcome. But see https://github.com/JuliaStats/Statistics.jl/pull/131 for an alternative "fix".

KronosTheLate commented 1 year ago

Alright, that seems like a better fix - to dispatch only on the case with numbers passed. So then I know that this issue is still noted and worked on somewhere - thanks ^_^

LilithHafner commented 1 year ago

Duplicate of #100