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

Add Base.FixKwargs (a la Base.Fix1) for more introspectable broadcasts #36093

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tkf
Copy link
Member

@tkf tkf commented May 31, 2020

This PR adds Base.Broadcast.FixKwargs which can be used to match and destructure dot-calls with keyword arguments. I think this is useful to have this as a public API.

Examples

Toy example

julia> using LazyArrays

julia> function f end
f (generic function with 0 methods)

julia> bc = @~ f.(1, 2; a = 3, b = 4);

julia> bc.f
(::Base.Broadcast.FixKwargs{typeof(f),Base.Iterators.Pairs{Symbol,Int64,Tuple{Symbol,Symbol},NamedTuple{(:a, :b),Tuple{Int64,Int64}}}}) (generic function with 1 method)

julia> bc.f.f === f
true

julia> (; bc.f.kwargs...)
(a = 3, b = 4)

julia> bc.args
(1, 2)

Example: keyword argument broadcasting

Keyword argument broadcasting #34737 can be implemented by users based on public API:

using Base.Broadcast: Broadcasted, FixKwargs, broadcastable

function withkwcall end
Broadcast.broadcasted(::typeof(withkwcall), x) = _fuse_kwcalls(x)

_fuse_kwcalls(x) = x
_fuse_kwcalls(bc::Broadcasted{Style}) where {Style} =
    _fuse_kwcalls(Broadcasted{Style}, bc.f, bc.args, bc.axes)

_fuse_kwcalls(BC, f, args, axes) = BC(f, map(_fuse_kwcalls, args), axes)
_fuse_kwcalls(BC, f::FixKwargs, args, axes) = BC(
    FlatKWCall{propertynames((; f.kwargs...))}(f.f),
    (map(_fuse_kwcalls, Tuple((; f.kwargs...)))..., map(_fuse_kwcalls, args)...),
    axes,
)

struct FlatKWCall{names,T} <: Function
    f::T
end
FlatKWCall{names}(f::T) where {names,T} = FlatKWCall{names,T}(f)

function (f::FlatKWCall{names})(args...) where {names}
    kw = NamedTuple{names}(args[1:length(names)])
    pos = args[length(names):end]
    return f.f(pos...; kw...)
end
julia> using LazyArrays

julia> f(args...; kwargs...) = (map(=>, 1:length(args), args)..., pairs(kwargs)...)
f (generic function with 1 method)

julia> withkwcall.(f.(1, [2], a = (@~ [1, 2] .+ 1), b = 3))
2-element Array{Tuple{Pair{Int64,Int64},Pair{Int64,Int64},Pair{Int64,Int64},Pair{Symbol,Int64},Pair{Symbol,Int64}},1}:
 (1 => 3, 2 => 1, 3 => 2, :a => 2, :b => 3)
 (1 => 3, 2 => 1, 3 => 2, :a => 3, :b => 3)

Type stability

While I'm at it, this PR also solves the problem with closures capturing types #23618. Before this PR, this test failed

julia/test/broadcast.jl

Lines 270 to 276 in 2290c4b

struct TypeWithKwargs end
TypeWithKwargs(a, args...; kwargs...) = TypeWithKwargs()
@testset "type inference with a type with kwargs" begin
f() = last.(tuple.([1], TypeWithKwargs.(1, 2; a = 3, b = 4)))[1]
@test @inferred(f()) === TypeWithKwargs()
end

and it now passes in this PR.

base/broadcast.jl Outdated Show resolved Hide resolved
@tkf tkf added the broadcast Applying a function over a collection label May 31, 2020
base/broadcast.jl Outdated Show resolved Hide resolved
base/broadcast.jl Outdated Show resolved Hide resolved
base/broadcast.jl Outdated Show resolved Hide resolved
@tkf
Copy link
Member Author

tkf commented Jun 1, 2020

Thanks, I reflected the comments.

@tkf
Copy link
Member Author

tkf commented Jun 1, 2020

It'd be nice if #36094 can be merged first so that FixKwargs can be listed in the documentation with Fix1 and Fix2.

@mbauman
Copy link
Member

mbauman commented Jun 1, 2020

I don't think order matters much — it's just one line of code in whichever PR goes second and it totally makes sense to include that line in either PR.

I'm 👍 with the broadcasting usage, changing the title to potentially attract a reviewer that cares more about our pseudo-currying functionality.

@mbauman mbauman changed the title Make Broadcasted with keyword arguments introspectable Add Base.FixKwargs (a la Base.Fix1) for more introspectable broadcasts Jun 1, 2020
@tkf
Copy link
Member Author

tkf commented Jun 1, 2020

It's just that @StefanKarpinski is a bit hesitant to add Fix1/Fix2 to the documentation. So I think it'd be bad to include FixKwargs before it's decided that it's OK to list Fix1/Fix2 in the documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
broadcast Applying a function over a collection
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants