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

replace equalto and occursin with curried isequal, ==, and in #26436

Merged
merged 1 commit into from
Mar 15, 2018

Conversation

JeffBezanson
Copy link
Member

@JeffBezanson JeffBezanson commented Mar 13, 2018

These were my silly idea, so I'll try to clean it up. I think this is nicer, frees up occursin, and easily generalizes to other functions.

This is really just currying on the second argument, but it's especially useful for comparisons since e.g. in(xs) checks whether something is in xs, and <(3) (if implemented) checks whether something is less than 3, etc. Dispatching on something like Curry2{typeof(in), ...} would be a bit obscure, so I opted for CompareTo{in, ...} instead.

Copy link
Member

@nalimilan nalimilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, though I still like @stevengj's proposal to use _ in x for this. My concern is that there's virtually no limit to the number of functions/operators which could be added a currying version like this (including in packages). And it will always be hard for users to remember which functions implement it and which don't.

For the bikeshedding, CompareTo sounds a bit weird for in. Even if more obscure, a general name referring only to currying could be fine. Anyway that's not exposed to most users (and it's not even exported). Maybe that type could even support any number of arguments, by wrapping a tuple to be splat rather than a single value (if there's no performance penalty).


(f::OccursIn)(y) = y in f.x
Create a function that compares its argument to `x` using [`==`](@ref); i.e. returns
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While you're at it, "returns a function equivalent to" would be more accurate.

test/arrayops.jl Outdated

@testset "findall(::OccursIn, b) for uncomparable element types" begin
@testset "findall(::In, b) for uncomparable element types" begin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a real syntax.

@stevengj
Copy link
Member

I think it would be a bit odd to implement this style in 1.0 and the underscore style in 1.1. Can we just make up our minds about the latter?

@piever
Copy link
Contributor

piever commented Mar 13, 2018

As a point in favor of the underscore syntax, probably the data people would find it more familiar as at least two packages (Query and JuliaDBMeta) use it already (it was reimplemented via macros): Lazy also reimplemented it with the @_ macro.

@JeffBezanson
Copy link
Member Author

I like the _ proposal but I don't see much harm in having both. isequal(x) is shorter than isequal(_,x). And the question is not whether this conflicts with _ syntax, but whether it's better than equalto and occursin.

@JeffBezanson JeffBezanson added the triage This should be discussed on a triage call label Mar 13, 2018
@StefanKarpinski
Copy link
Member

I like the in(collection) and isequal(value) currying, not sure about the Curry2 type.

@JeffBezanson
Copy link
Member Author

The idea would be to share Curry2 with f(_, x) syntax in the future.

@StefanKarpinski
Copy link
Member

Perhaps we could use _ in { } in some clever way for that?

@mbauman
Copy link
Member

mbauman commented Mar 13, 2018

I definitely like that this makes ==(-0.0) and isequal(-0.0) clearer. I can never remember which one equalto implements.

@jw3126
Copy link
Contributor

jw3126 commented Mar 14, 2018

I find the proposed methods confusing and would be much more happy with only e.g. (_ == x). One reason that was mentioned yet is the following. We currently have:

+(args...) = sum(args)
*(args...) = prod(args)

This gives for example

+(3) = 3

OTOH from this proposal I would expect

+(3) = x -> x + 3

I think implicit currying is confusing in a language that also allows variable number of arguments.

@StefanKarpinski
Copy link
Member

Triage is on board with trying this. Some observations:

  1. This only makes sense for binary operations.
  2. It isn't technically currying but rather partial application; accordingly it would be both clearer and more accurate to write Fix2 to indicate fixing the second argument via partial application.
  3. It only reads correctly for operations where we use verb(subject, object) order, which is the case for in and < for example and for those it should always do specialization on the second argument.

So in short, I think we should limit this convention to binary verb(subject, object) functions and always do specialization on the object.

@stevengj
Copy link
Member

stevengj commented Mar 14, 2018

Can I suggest a more general type than Curry2 or Fix2, to make this easier to generalize to underscore syntax?

In particular, how about something like struct FixFunction{N,F,i,T}; f::F; x::T; end where N is the new number of arguments, f is the function, i is an M-tuple of the positional indices of the "fixed" arguments x, and T is a tuple type of M types. So that _ < 3 becomes FixFunction{1,typeof(<),(2,),Tuple{Int}}(<,(3,)).

@mbauman
Copy link
Member

mbauman commented Mar 14, 2018

That was considered on our call — Fix2 is really easy to type-alias to a more general form in the future. A more general form that we get wrong is much harder to patch around.

@StefanKarpinski StefanKarpinski removed the triage This should be discussed on a triage call label Mar 15, 2018
@StefanKarpinski StefanKarpinski added this to the 1.0 milestone Mar 15, 2018
@JeffBezanson JeffBezanson merged commit 71fbc70 into master Mar 15, 2018
@JeffBezanson JeffBezanson deleted the jb/curriedcomparisons branch March 15, 2018 18:28
@StefanKarpinski
Copy link
Member

After sleeping on it, triage has decided to go for this but only do this for strictly binary operators (i.e. only two-arg methods) where the second argument is the object of the verb, which limits the scope creep of this considerably and restricts the convention to cases where it is linguistically intuitive.

* New function `equalto(x)`, which returns a function that compares its argument to `x`
using `isequal` ([#23812]).
* `isequal`, `==`, and `in` have one argument "curried" forms. For example `isequal(x)`
returns a function that compares its argument to `x` using `isequal` ([#23812]).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR number should have been updated to 26436 here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adjusted in #26480

@stevengj
Copy link
Member

stevengj commented Mar 15, 2018

@mbauman, a problem here is that Fix2 has a field x that is the curried argument. Any generalization to multiple parameters will almost certainly have to change this to a tuple, at which point Fix2 won't simply be a type alias.

Can the x member be changed to a Tuple{T}? Otherwise any more general partial evaluation scheme (ala #24990) will have to introduce a breaking change in order to interact nicely with this feature.

@mbauman
Copy link
Member

mbauman commented Mar 15, 2018

Or we add a deprecated property accessor (either via getproperty or a dedicated function) if we really want to make property access non-breaking?

@JeffBezanson
Copy link
Member Author

The generalized partial application type seems so complicated to me that you might as well encode the AST in the type at that point (as prototyped by @vtjnash). We can just have a rule that f(_, x) lowers to a Fix2 and f(x, _) lowers to a Fix1, and other cases do other things.

@ararslan ararslan added the breaking This change will break code label Mar 15, 2018
@ararslan
Copy link
Member

equalto and occursin should have had deprecations; this now causes UndefVarErrors for code that's already been updated to work on 0.7.

@JeffBezanson
Copy link
Member Author

Ok. Can you add them?

@ararslan
Copy link
Member

#26480

garrison added a commit to garrison/UniqueVectors.jl that referenced this pull request Mar 16, 2018
Still waiting on Compat support for this to work on julia 0.6 as well.
fredrikekre added a commit to JuliaLang/Compat.jl that referenced this pull request Mar 16, 2018
Add one argument ``curried'' forms of `isequal`, `==` and `in`.
Keep the following definitions to not break Compat on julia v0.6:
 - Compat.equalto(x) = isequal(x)
 - Compat.occursin(x) = in(x)
fredrikekre added a commit to JuliaLang/Compat.jl that referenced this pull request Mar 16, 2018
Add one argument ``curried'' forms of `isequal`, `==` and `in`.
Keep the following definitions to not break Compat on julia v0.6:
 - const EqualTo = Fix2{typeof(isequal)}
 - Compat.equalto(x) = isequal(x)
 - const OccursIn = Fix2{typeof(in)}
 - Compat.occursin(x) = in(x)
fredrikekre added a commit to JuliaLang/Compat.jl that referenced this pull request Mar 16, 2018
- Add one argument ``curried'' forms of `isequal`, `==` and `in`.

- Keep the following definitions to not break Compat on julia v0.6:
   - const EqualTo{T} = Fix2{typeof(isequal),T}
   - equalto(x) = isequal(x)
   - const OccursIn{T} = Fix2{typeof(in),T}
   - occursin(x) = in(x)
fredrikekre added a commit to JuliaLang/Compat.jl that referenced this pull request Mar 16, 2018
- Add one argument ``curried'' forms of `isequal`, `==` and `in`.

- Keep the following definitions to not break Compat on julia v0.6:
   - const EqualTo{T} = Fix2{typeof(isequal),T}
   - equalto(x) = isequal(x)
   - const OccursIn{T} = Fix2{typeof(in),T}
   - occursin(x) = in(x)
fredrikekre added a commit to JuliaLang/Compat.jl that referenced this pull request Mar 17, 2018
- Add one argument ``curried'' forms of `isequal`, `==` and `in`.

- Keep the following definitions to not break Compat on julia v0.6:
   - const EqualTo{T} = Fix2{typeof(isequal),T}
   - equalto(x) = isequal(x)
   - const OccursIn{T} = Fix2{typeof(in),T}
   - occursin(x) = in(x)
fredrikekre added a commit to JuliaLang/Compat.jl that referenced this pull request Mar 17, 2018
- Add one argument ``curried'' forms of `isequal`, `==` and `in`.

- Keep the following definitions to not break Compat on julia v0.6:
   - const EqualTo{T} = Fix2{typeof(isequal),T}
   - equalto(x) = isequal(x)
   - const OccursIn{T} = Fix2{typeof(in),T}
   - occursin(x) = in(x)
@rapus95
Copy link
Contributor

rapus95 commented Jun 11, 2018

was there a particular reason why you didn't add isa(T::DataType) aswell?

@StefanKarpinski
Copy link
Member

No, that would be a good one as well.

@andyferris
Copy link
Member

Not sure where to write this... but after getting used to isequal(x) and ==(x) I was surprised not to see isless or < (and >, <=, >=).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This change will break code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants