Allow user-defined functors to be passed to more methods in Base #12035

malmaud · 2015-07-06T18:33:48Z

Closes #12012

Closes JuliaLang#12012

yuyichao · 2015-07-06T18:47:11Z

What are the places left that uses Callable and do we still need it?

malmaud · 2015-07-06T18:50:16Z

Only in get and get!. I was getting ambiguity warnings with get!(o::ObjectIdDict, key, default) = (o[key] = get(o, key, default)) in dict.jl; haven't looked into it further.

tkelman · 2015-07-06T19:51:16Z

base/operators.jl

@@ -189,7 +189,7 @@ copy(x::Union{Symbol,Number,AbstractString,Function,Tuple,LambdaStaticData,
              TopNode,QuoteNode,DataType,Union}) = x

 # function pipelining
-|>(x, f::Callable) = f(x)
+|>(x, f) = f(x)


Won't this conflict with the deprecated piping of Cmd objects via |> ? I don't think we can make this change until after that deprecation gets removed.

I don't think so, since Cmd piping will still invoke the more specific |>(src::Base.AbstractCmd, dest::Base.AbstractCmd).

hm okay, well if no ambiguities then I guess we can see what happens

yuyichao · 2015-07-06T19:58:17Z

Only in get and get!. I was getting ambiguity warnings with get!(o::ObjectIdDict, key, default) = (o[key] = get(o, key, default)) in dict.jl; haven't looked into it further.

I guess the get interface needs to distinguish between callable and non-callable object and keeping it only limited to function and types would probably be the simplest solusion.

malmaud · 2015-07-07T15:38:35Z

Maybe it's best to just change the get interface to not be ambiguous wrt ObjectIdDict, and then Callable can be removed completely. I don't have a good sense of how often that interface is used in packages and therefore how breaking it would be.

toivoh · 2015-07-08T05:19:38Z

Agreed, now that everything is potentially callable, we should get rid of Callable entirely and rework those apis that relied on it. Otherwise we will leave some nasty surprises for people who use objects with call overloading.

malmaud · 2015-07-10T00:26:45Z

What should we do about the ambiguity that will exist between get{K,V}(default::Callable, h::Dict{K,V}, key) and get{K,V}(h::Dict{K,V}, key, default)?

kmsquire · 2015-07-10T04:21:43Z

Perhaps change the name of the first?

On Thursday, July 9, 2015, Jonathan Malmaud notifications@github.com
wrote:

What should we do about the ambiguity that will exist between get{K,V}(default::Callable,
h::Dict{K,V}, key) and get{K,V}(h::Dict{K,V}, key, default)?

—
Reply to this email directly or view it on GitHub
#12035 (comment).

nalimilan · 2015-07-10T08:49:44Z

OTOH, restricting these methods to Callable limits ambiguities (as can be seen above) and means that you'll get a no method error pretty early when a call doesn't make sense, which is always good. Shouldn't objects that implement call inherit from Callable instead?

timholy · 2015-07-10T11:50:57Z

Given that we don't have multiple inheritance, that would be a rather extreme constraint on type hierarchies. Much better to ditch Callable altogether.

ScottPJones · 2015-07-10T11:57:36Z

Would this be a case where "Holy" Traits could come to the rescue?

nalimilan · 2015-07-10T13:19:06Z

Yeah, actually I had in mind a kind of a trait (which the name Callable evokes).

yuyichao · 2015-07-10T13:22:14Z

I strongly disagree that using a trait here is a good API.
It'll easily break if someone decided to make a object callable.

ScottPJones · 2015-07-10T13:28:16Z

@yuyichao Could you elaborate please?

malmaud · 2015-07-10T19:24:36Z

It seems get(default::Callable, h::Dict{K,V}, key) and friends will have to be eliminated, since without Callable it will be ambiguous with any method of the form get(::T, key, default). But that will be a breaking change that eliminates using do syntax with get. Maybe for the time being it's best to not touch those methods, and then this PR will be non-breaking.

timholy · 2015-07-10T23:28:42Z

What about inserting a deprecation now and having it call a function of a new name? Otherwise you're stuck with it through 0.5, too.

StefanKarpinski · 2015-07-11T12:49:01Z

Long ago I proposed a d[k] ?= v syntax for this operation. Maybe it's time to have that.

toivoh · 2015-07-12T08:16:46Z

+1

kmsquire · 2015-07-12T08:30:11Z

I think that syntax is fine, but I don't see how it solves the Callable issue...

malmaud · 2015-07-14T16:42:50Z

@timholy You're proposing introducing a new function that implements the behavior that get(::Callable,...) currently has? That does seem to be the only solution, short of changing how do functions work to allow them to still be used with get while avoiding defining get(::Callable

timholy · 2015-07-14T18:15:15Z

Yeah. It's basically the same thing that @kmsquire was saying, I think.

JeffBezanson · 2015-07-18T03:41:22Z

I envision making Function an abstract type, for objects that are primarily functions. That doesn't directly solve this problem, but what you can do is annotate arguments with ::Function or ::Callable when needed to avoid ambiguities. Then at worst the caller needs to wrap the intended argument in an anonymous function, which presumably will be fast at some point :)

malmaud · 2015-07-19T14:33:47Z

Seems like it could be a bit janky: I have a callable object f that can't derive from Function because of the lack of multiple inheritance. I then have to pass it to a higher-order function as (args...)->f(args...) to get it to dispatch properly. I guess we could have something like asfunction(f) which automatically does that to allay the pain.

yuyichao · 2015-07-19T14:36:08Z

Feels like #5571 (a little bit).

In any case, should we merge the non-ambigious part of this PR?

JeffBezanson · 2015-07-20T16:07:19Z

base/reduce.jl

@@ -186,7 +186,7 @@ sum_pairwise_blocksize(::Abs2Fun) = 4096
 mapreduce_impl(f, op::AddFun, A::AbstractArray, ifirst::Int, ilast::Int) =
    mapreduce_pairwise_impl(f, op, A, ifirst, ilast, sum_pairwise_blocksize(f))

-sum(f::Union{Callable,Func{1}}, a) = mapreduce(f, AddFun(), a)
+sum(f, a) = mapreduce(f, AddFun(), a)


This seems weird to me, given the existence of sum(array, dims). I think ideally this definition would not exist at all.
count is ok, since its first argument is always a function.

JeffBezanson · 2015-07-20T21:49:59Z

Fixed on master.

Allow user-defined functors to be passed to more methods in Base

5c4a0a7

Closes JuliaLang#12012

tkelman reviewed Jul 6, 2015
View reviewed changes

vtjnash mentioned this pull request Jul 15, 2015

julep/rfc: proposal for unified Dict get/insert/has interface #12157

Open

yuyichao mentioned this pull request Jul 19, 2015

Can't broadcast non-Function Callable #12216

Closed

JeffBezanson reviewed Jul 20, 2015
View reviewed changes

JeffBezanson closed this Jul 20, 2015

tkelman mentioned this pull request Jul 22, 2015

RFC: Make any and all short-circuiting #11774

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow user-defined functors to be passed to more methods in Base #12035

Allow user-defined functors to be passed to more methods in Base #12035

malmaud commented Jul 6, 2015

yuyichao commented Jul 6, 2015

malmaud commented Jul 6, 2015

tkelman Jul 6, 2015

malmaud Jul 6, 2015

tkelman Jul 6, 2015

yuyichao commented Jul 6, 2015

malmaud commented Jul 7, 2015

toivoh commented Jul 8, 2015

malmaud commented Jul 10, 2015

kmsquire commented Jul 10, 2015

nalimilan commented Jul 10, 2015

timholy commented Jul 10, 2015

ScottPJones commented Jul 10, 2015

nalimilan commented Jul 10, 2015

yuyichao commented Jul 10, 2015

ScottPJones commented Jul 10, 2015

malmaud commented Jul 10, 2015

timholy commented Jul 10, 2015

StefanKarpinski commented Jul 11, 2015

toivoh commented Jul 12, 2015 via email

kmsquire commented Jul 12, 2015

malmaud commented Jul 14, 2015

timholy commented Jul 14, 2015

JeffBezanson commented Jul 18, 2015

malmaud commented Jul 19, 2015

yuyichao commented Jul 19, 2015

JeffBezanson Jul 20, 2015

JeffBezanson commented Jul 20, 2015

Allow user-defined functors to be passed to more methods in Base #12035

Allow user-defined functors to be passed to more methods in Base #12035

Conversation

malmaud commented Jul 6, 2015

yuyichao commented Jul 6, 2015

malmaud commented Jul 6, 2015

tkelman Jul 6, 2015

Choose a reason for hiding this comment

malmaud Jul 6, 2015

Choose a reason for hiding this comment

tkelman Jul 6, 2015

Choose a reason for hiding this comment

yuyichao commented Jul 6, 2015

malmaud commented Jul 7, 2015

toivoh commented Jul 8, 2015

malmaud commented Jul 10, 2015

kmsquire commented Jul 10, 2015

nalimilan commented Jul 10, 2015

timholy commented Jul 10, 2015

ScottPJones commented Jul 10, 2015

nalimilan commented Jul 10, 2015

yuyichao commented Jul 10, 2015

ScottPJones commented Jul 10, 2015

malmaud commented Jul 10, 2015

timholy commented Jul 10, 2015

StefanKarpinski commented Jul 11, 2015

toivoh commented Jul 12, 2015 via email

kmsquire commented Jul 12, 2015

malmaud commented Jul 14, 2015

timholy commented Jul 14, 2015

JeffBezanson commented Jul 18, 2015

malmaud commented Jul 19, 2015

yuyichao commented Jul 19, 2015

JeffBezanson Jul 20, 2015

Choose a reason for hiding this comment

JeffBezanson commented Jul 20, 2015