-
Notifications
You must be signed in to change notification settings - Fork 40
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 init kwarg to mean/median to match other reductions in Base #105
Comments
Makes sense. Would you be willing to make a pull request? |
Surely |
Ah good point. @non-Jedi that wouldn't work for your use case then. |
For both
I'd argue that we should just go ahead and specify that |
I think this is unspecified because for technical (performance) reasons sometimes we need to use julia> sum([1], init=missing)
missing
julia> sum([1], init=100)
101
AFAICT this problem essentially happens with |
To be clear, I wasn't suggesting we change and specify the meaning of
I think the problem is more the general awkwardness in julia of working with possibly empty collections--especially non-materialized ones--and not specific to working with In my ideal world, I think Note that just adding an |
See AccessorsExtra for a composable approach to this problem: julia> using AccessorsExtra
julia> maybe(first)([1,2,3])
1
julia> maybe(first)([])
# nothing
julia> maybe(@o first(_).a)([(a=1,)])
1
julia> maybe(@o first(_).a)([(b=1,)])
# nothing
julia> maybe(@o first(_).a)([])
# nothing
# convenience functions using the same machinery:
julia> x = [1,2,3];
julia> @oget only(x) NaN
NaN
julia> x = [1];
julia> @oget only(x) NaN
1
julia> x = [];
julia> @oget only(x) NaN
NaN It fundamentally uses |
(#132 is a related issue with related discussion and possibly duplicate) |
Funny, that's precisely how I ended up deep down this rabbit hole. See JuliaLang/julia#53945 for where I think the consensus is pointing us. |
Thanks @mbauman. I think after reading through some of the interlinked issues and PRs that I'm convinced adding an From @mbauman's post immediately following, point 2 is not a concern since Possible solutions I see right now:
The discussion in #132 seemed to heavily prefer solution 5 over solution 2, and I'm not sure I really understand why. @aplavin could you comment on this since you were a proponent of solution 5? |
The advantage of a wrapper function is that it immediately works with any function ( |
Basically what @nalimilan says, to be able to use it with any kind of function. Mean is nothing special here, and it would be much cleaner to add a single wrapper/postprocessing function than provide options to handle empty collections in each aggregation. It doesn't preclude having the most commonly used options added directly to the most commonly used functions, like mean. The same story as with the |
The absence of an
init
kwarg like is available withreduce
andsum
makesmean
compose less well with e.g.skipmissing
. I'm finding myself usinglet y=collect(skipmissing(x)); isempty(y) ? missing : mean(y) end
when I should be able to just saymean(skipmissing(x); init=missing)
.There may be other reductions than
mean
andmedian
in Statistics.jl that also should haveinit
added, but those are the two I've needed (median isn't quite a reduction technically, but it's close enough that aninit
kwarg would be useful).The text was updated successfully, but these errors were encountered: