Add mergewith[!](combine, dicts...)#34296
Conversation
|
I would prefer it if the function form were called something like |
|
That would be great. Can we also have the curried version |
|
@andyferris @oxinabox Maybe you are interested in this discussion as this increases dictionary API. |
|
I added |
|
Is the probem with Is the odd thing about this method rather the "implict reduction" over varargs dictionaries? The (long) discussion at #32860 seems to at times suggest such implicit folding/reducing might be superceded by an explicit one - leaving here just |
|
I agree that it sounds problematic to introduce AFAICT there's no ambiguity between |
Yes, for example, In principle, we can do this without introducing right(_, x) = x
merge!(dest::MyType, other, rest...) = _merge_impl(right, dest, other, others...)
merge!(combine, dest::MyType, other, rest...) = _merge_impl(combine, dest, other, others...)
# Disambiguation:
merge!(dest::MyType, other::MyType, rest...) = _merge_impl(right, dest, other, others...)I think it's OK. But it's ugly; especially the disambiguation
I think we only need this for "monoid-like" functions; i.e., the ones with signatures " |
|
I have to admit, the callable amibuguity is a bit annoying. You can't add a type to the We do have a pattern for dealing with these things - traits. Speculatively, we could define in export iscallable
iscallable(::Any) = false
iscallable(::Function) = true
iscallable(::Type) = trueand query |
Actually, doesn't |
|
I agree defining trait is a possible approach, but why bring such an "advanced" solution when you have a dumb solution? It also makes it harder to know what I think it's a nice property that you can tell what
I think it does. But I think the list of |
|
PyCall are indeed the most problematic example, but it's use with |
|
I don't understand why PyCall example is hypothetical. Personally, I'd use it if it were usable. Also, that's exactly the reason why the type restriction of In Julia, everything is potentially callable. I think it'd be nice if |
@tkf - something in particular must have motivated you to create this PR - it is often helpful to understand these cases, so I feel the question is fair. Was there something you were trying to do that didn't work for you?
Agreed. |
|
OK. That's a fair point. I just didn't want to bring it up as it's not a clean example. It came up when I tried something like julia> using Transducers: Map, reducingfunction
julia> rf = reducingfunction(Map(x -> 2x), +)
Reduction(
Map(Main.λ❓),
BottomRF(
+))
julia> merge(rf, Dict(:a => 1), Dict(:a => 1))
ERROR: MethodError: no method matching merge(::Transducers.Reduction{Map{var"#3#4"},Transducers.BottomRF{typeof(+)}}, ::Dict{Symbol,Int64}, ::Dict{Symbol,Int64})
Closest candidates are:
merge(::OrderedCollections.OrderedDict, ::AbstractDict...) at /home/takafumi/.julia/packages/OrderedCollections/E21Rb/src/ordered_dict.jl:434
merge(::DataStructures.SortedDict{K,D,Ord}, ::AbstractDict{K,D}...) where {K, D, Ord<:Base.Order.Ordering} at /home/takafumi/.julia/packages/DataStructures/iymwN/src/sorted_dict.jl:598
merge(::AbstractDict, ::AbstractDict...) at abstractdict.jl:284
...
Stacktrace:
[1] top-level scope at REPL[9]:1
julia> merge((args...) -> rf(args...), Dict(:a => 1), Dict(:a => 1))
Dict{Symbol,Int64} with 1 entry:
:a => 3But I thought that it was not a good example as it is "fixable" and, being the library author, arguably I should. I can make whatever returned by However, I don't think tweaking the type hierarchy is the right approach. |
|
With multiple dispatch, just as the meaning of a function is important to get consistent, it's also important to get the meanings of the argument slots consistent when possible. It's not ideal for a function to treat an argument in a fundamentally different way based on its type: e.g. "if this is a Function, then call it on pairs of values, otherwise assume it's a container providing the values to use". Given the meaning of |
Co-Authored-By: Jameson Nash <vtjnash@gmail.com>
Co-Authored-By: Jeff Bezanson <jeff.bezanson@gmail.com>
after #34296 this method is redundant, and it's also undocumented. according to #43491 (comment), `Callable` should not be used anymore, so it might make sense to remove from `Base` where feasible.
after #34296 this method is redundant, and it's also undocumented. according to #43491 (comment), `Callable` should not be used anymore, so it might make sense to remove from `Base` where feasible. (cherry picked from commit 5957579)
Can we remove the restriction
::Functioninmerge[!](combine::Function, d::AbstractDict, others::AbstractDict...)? Was this there to disambiguate the case where the first argument isAbstractDictbut also is callable? Or was it there to allowmerge[!](d::MyDict, ::Any)?