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

Support keyword arguments in mean #21

Closed
wants to merge 1 commit into from
Closed

Support keyword arguments in mean #21

wants to merge 1 commit into from

Conversation

BatyLeo
Copy link
Member

@BatyLeo BatyLeo commented Aug 1, 2024

Support for kwargs was missing in the mean method with a preprocessing function f.

@BatyLeo BatyLeo requested a review from gdalle August 1, 2024 13:50
Copy link

codecov bot commented Aug 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Files Coverage Δ
src/distribution.jl 100.00% <100.00%> (ø)

@gdalle
Copy link
Member

gdalle commented Aug 1, 2024

I didn't add it because the Statistics version doesn't support kwargs, so it felt counter-intuitive that ours would? Users can always fix kwargs themselves.

julia> using Statistics

julia> f(x; y) = x + y
f (generic function with 1 method)

julia> mean(f, 1:3; y=2)
ERROR: MethodError: no method matching mean(::typeof(f), ::UnitRange{Int64}; y::Int64)

Closest candidates are:
  mean(::Any, ::AbstractArray; dims) got unsupported keyword argument "y"
   @ Statistics ~/.julia/juliaup/julia-1.10.4+0.x64.apple.darwin14/share/julia/stdlib/v1.10/Statistics/src/Statistics.jl:104
  mean(::Any, ::Any) got unsupported keyword argument "y"
   @ Statistics ~/.julia/juliaup/julia-1.10.4+0.x64.apple.darwin14/share/julia/stdlib/v1.10/Statistics/src/Statistics.jl:61
  mean(::Any) got unsupported keyword argument "y"
   @ Statistics ~/.julia/juliaup/julia-1.10.4+0.x64.apple.darwin14/share/julia/stdlib/v1.10/Statistics/src/Statistics.jl:44

@BatyLeo
Copy link
Member Author

BatyLeo commented Aug 1, 2024

I added this because I needed it for InferOpt.
By fixing kwargs themselves, do you mean something like this:

f(x; y) = x + y

mean(x -> f(x; y=2), 1:3)

@BatyLeo
Copy link
Member Author

BatyLeo commented Aug 1, 2024

Alternatively users can use FixKwargs, I agree it's not needed then.

@BatyLeo BatyLeo closed this Aug 1, 2024
@gdalle
Copy link
Member

gdalle commented Aug 1, 2024

DifferentiableExpectation can expose FixKwargs as part of the public (unexported) API if it helps

@BatyLeo
Copy link
Member Author

BatyLeo commented Aug 1, 2024

I think it's a good idea, I'm already using it extensively in the current InferOpt overhaul

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants