Infix functions for Set #515

Merged
merged 2 commits into from Nov 17, 2014

Conversation

Projects
None yet
6 participants
@c-cube
Member

c-cube commented Jan 20, 2014

see #250

@thelema

This comment has been minimized.

Show comment Hide comment
@thelema

thelema Jan 21, 2014

Member

Good job changing ^ and & to ||| and &&&; I prefer these.

Member

thelema commented Jan 21, 2014

Good job changing ^ and & to ||| and &&&; I prefer these.

@hcarty

This comment has been minimized.

Show comment Hide comment
@hcarty

hcarty Jan 21, 2014

Contributor

Agreed

Contributor

hcarty commented Jan 21, 2014

Agreed

@gasche

This comment has been minimized.

Show comment Hide comment
@gasche

gasche Jan 21, 2014

Member

As always for infix operators that overload polymorphic comparisons, I'm left wondering whether >. or something else wouldn't be a better choice. That said we could consider this is consistent with having Set.compare, etc. Otherwise I'm fine with the commit and the operator choices.

Member

gasche commented Jan 21, 2014

As always for infix operators that overload polymorphic comparisons, I'm left wondering whether >. or something else wouldn't be a better choice. That said we could consider this is consistent with having Set.compare, etc. Otherwise I'm fine with the commit and the operator choices.

@c-cube

This comment has been minimized.

Show comment Hide comment
@c-cube

c-cube Jan 22, 2014

Member

@gasche this kind of Infix modules, I think, is not to be opened in a large scope, but in a local context for Set-specific computations. In this case. In this case, shadowing polymorphic comparison operators doesn't sound too dangerous to me.

In favor of merging?

Member

c-cube commented Jan 22, 2014

@gasche this kind of Infix modules, I think, is not to be opened in a large scope, but in a local context for Set-specific computations. In this case. In this case, shadowing polymorphic comparison operators doesn't sound too dangerous to me.

In favor of merging?

@gasche

This comment has been minimized.

Show comment Hide comment
@gasche

gasche Jan 23, 2014

Member

I cannot say that I am. My gut feeling is that overriding existing operators is bad, and overriding the polymorphic ones is "the worst of the bad". I won't veto this if there is an large consensus, but I'm interested in what you and hcarty would think of using a suffixed variant of those operators (I think we should pick the same suffix in all the Infix modules, it could be . or something else, for example ! as in Num).

It's not a new debate, see PR#5458 against the distribution's stdlib.

Member

gasche commented Jan 23, 2014

I cannot say that I am. My gut feeling is that overriding existing operators is bad, and overriding the polymorphic ones is "the worst of the bad". I won't veto this if there is an large consensus, but I'm interested in what you and hcarty would think of using a suffixed variant of those operators (I think we should pick the same suffix in all the Infix modules, it could be . or something else, for example ! as in Num).

It's not a new debate, see PR#5458 against the distribution's stdlib.

@c-cube

This comment has been minimized.

Show comment Hide comment
@c-cube

c-cube Jan 23, 2014

Member

Wouldn't an intermediate solution be to have generic operators (like +., >. and so on, but not colliding with Float. So something like ++, >> for instance) that aren't also polymorphic operators? Then, one could locally open Infix modules, use infix operators with a consistent convention across modules, but without shadowing the polymorphic operators.

Member

c-cube commented Jan 23, 2014

Wouldn't an intermediate solution be to have generic operators (like +., >. and so on, but not colliding with Float. So something like ++, >> for instance) that aren't also polymorphic operators? Then, one could locally open Infix modules, use infix operators with a consistent convention across modules, but without shadowing the polymorphic operators.

@gasche

This comment has been minimized.

Show comment Hide comment
@gasche

gasche Jan 23, 2014

Member

I don't understand. What do you mean by "generic" if not "polymorphic"? My suggestion is to use >., +. consistently across modules for operators that we're tempted to call > or +. (But I would be ready to let + be used, as it's not polymorphic and therefore slightly less pervasive in actual code.)

Member

gasche commented Jan 23, 2014

I don't understand. What do you mean by "generic" if not "polymorphic"? My suggestion is to use >., +. consistently across modules for operators that we're tempted to call > or +. (But I would be ready to let + be used, as it's not polymorphic and therefore slightly less pervasive in actual code.)

@hcarty

This comment has been minimized.

Show comment Hide comment
@hcarty

hcarty Jan 23, 2014

Contributor

Please don't mix overriding >. and + (no suffix on the +). That would make it overly difficult to follow which operators are acting in which capacity. Consistency is important - either all of these common operators should have the same suffix or none of them should have a suffix.

Contributor

hcarty commented Jan 23, 2014

Please don't mix overriding >. and + (no suffix on the +). That would make it overly difficult to follow which operators are acting in which capacity. Consistency is important - either all of these common operators should have the same suffix or none of them should have a suffix.

@gasche

This comment has been minimized.

Show comment Hide comment
@gasche

gasche Jan 23, 2014

Member

Why not; let's consider "common operators" those that are already present in the compiler's Pervasives library, and consider that my (alternative) proposal is to consistently suffix those. What do you think?

Member

gasche commented Jan 23, 2014

Why not; let's consider "common operators" those that are already present in the compiler's Pervasives library, and consider that my (alternative) proposal is to consistently suffix those. What do you think?

@hcarty

This comment has been minimized.

Show comment Hide comment
@hcarty

hcarty Jan 23, 2014

Contributor

I think your (@gasche's) alternate proposal is the better option. I've gone back and forth a lot on this, M.( + ) vs M.( +. ). Your point in the PR#5458 regarding indexing with integers swayed me in favor of suffixes.

As far as which suffix to choose, I think . is a reasonable choice. It is already established in the stdlib for floats. It does make code more verbose when performing floating point operations in a open module context. In that context being explicit with Float.(a +. b) is probably a good thing to do anyway.

Contributor

hcarty commented Jan 23, 2014

I think your (@gasche's) alternate proposal is the better option. I've gone back and forth a lot on this, M.( + ) vs M.( +. ). Your point in the PR#5458 regarding indexing with integers swayed me in favor of suffixes.

As far as which suffix to choose, I think . is a reasonable choice. It is already established in the stdlib for floats. It does make code more verbose when performing floating point operations in a open module context. In that context being explicit with Float.(a +. b) is probably a good thing to do anyway.

@c-cube

This comment has been minimized.

Show comment Hide comment
@c-cube

c-cube Jan 23, 2014

Member

I agree too, dot suffix is easy to type and not too obstrusive. Should we also switch to &. and |. for logic-like operations (here union and intersection)?

Member

c-cube commented Jan 23, 2014

I agree too, dot suffix is easy to type and not too obstrusive. Should we also switch to &. and |. for logic-like operations (here union and intersection)?

@UnixJunkie

This comment has been minimized.

Show comment Hide comment
@UnixJunkie

UnixJunkie Jan 24, 2014

Member

"overriding existing operators is bad" said @gasche and I fully agree with that:
don't do this, it will make code less readable and code readability is one of the
jewels of OCaml code.
To me + or +. and * or *. are always integer or float operators,
they should not have another meaning.
So, dot seams like a bad suffix to me since it is already in use.

Member

UnixJunkie commented Jan 24, 2014

"overriding existing operators is bad" said @gasche and I fully agree with that:
don't do this, it will make code less readable and code readability is one of the
jewels of OCaml code.
To me + or +. and * or *. are always integer or float operators,
they should not have another meaning.
So, dot seams like a bad suffix to me since it is already in use.

@UnixJunkie

This comment has been minimized.

Show comment Hide comment
@UnixJunkie

UnixJunkie Jan 24, 2014

Member

From code I have read from other programmers, I like new operators such as ||| or &&&: it is bold
and explicit, and when you read them you know it is not a standard operator.

Member

UnixJunkie commented Jan 24, 2014

From code I have read from other programmers, I like new operators such as ||| or &&&: it is bold
and explicit, and when you read them you know it is not a standard operator.

@c-cube

This comment has been minimized.

Show comment Hide comment
@c-cube

c-cube Jan 24, 2014

Member

Well then, maybe triplicating every operator will satisfy everyone: +++, ---, >>>, |||, and so on ^^

Member

c-cube commented Jan 24, 2014

Well then, maybe triplicating every operator will satisfy everyone: +++, ---, >>>, |||, and so on ^^

@gasche

This comment has been minimized.

Show comment Hide comment
@gasche

gasche Jan 24, 2014

Member

Do you see yourself using this for Complex, Int64 as well? I'm not saying that the infix operator for set union should necessarily be the same as addition, I'm just wondering about whether people would find that choice pervasively acceptable, or if it's just "ok for sets as they're weird anyway".

Member

gasche commented Jan 24, 2014

Do you see yourself using this for Complex, Int64 as well? I'm not saying that the infix operator for set union should necessarily be the same as addition, I'm just wondering about whether people would find that choice pervasively acceptable, or if it's just "ok for sets as they're weird anyway".

@hcarty

This comment has been minimized.

Show comment Hide comment
@hcarty

hcarty Jan 24, 2014

Contributor

I still think the dot suffix is the better option. Tripling operator characters will quickly obscure what's going on in code. Some options using Int64 as an example:

Int64.(1L + x / 4L * y)
Int64.(1L +. x /. 4L *. y)
Int64.(1L +/ x // 4L */ y)
Int64.(1L +! x /! 4L *! y)
Int64.(1L +: x /: 4L *: y) (* camlp4 issues with <: *)
Int64.(1L ++ x // 4L ** y) (* Breaks order of operations *)
Int64.(1L +++ x /// 4L *** y)

I don't think we have a good operator suffix option other than . or no suffix which is unobtrusive and avoids other syntax incompatibilities.

Contributor

hcarty commented Jan 24, 2014

I still think the dot suffix is the better option. Tripling operator characters will quickly obscure what's going on in code. Some options using Int64 as an example:

Int64.(1L + x / 4L * y)
Int64.(1L +. x /. 4L *. y)
Int64.(1L +/ x // 4L */ y)
Int64.(1L +! x /! 4L *! y)
Int64.(1L +: x /: 4L *: y) (* camlp4 issues with <: *)
Int64.(1L ++ x // 4L ** y) (* Breaks order of operations *)
Int64.(1L +++ x /// 4L *** y)

I don't think we have a good operator suffix option other than . or no suffix which is unobtrusive and avoids other syntax incompatibilities.

@c-cube

This comment has been minimized.

Show comment Hide comment
@c-cube

c-cube Jan 24, 2014

Member

I noticed that Zarith uses the conventional integer operators for addition, product, etc. but not for polymorphic operators like comparison and equality. Guess that overloading > and = is really bad...
I'm still quite convinced that using dot-suffixed operators, as @hcarty defends, is quite good, as is the Num-like / suffixing. Shall we move on on this issue? :)

Maybe that will also go in 3.0...

Member

c-cube commented Jan 24, 2014

I noticed that Zarith uses the conventional integer operators for addition, product, etc. but not for polymorphic operators like comparison and equality. Guess that overloading > and = is really bad...
I'm still quite convinced that using dot-suffixed operators, as @hcarty defends, is quite good, as is the Num-like / suffixing. Shall we move on on this issue? :)

Maybe that will also go in 3.0...

@UnixJunkie

This comment has been minimized.

Show comment Hide comment
@UnixJunkie

UnixJunkie Jan 27, 2014

Member

My suggestion was for sets indeed, not for complex or int64.

Member

UnixJunkie commented Jan 27, 2014

My suggestion was for sets indeed, not for complex or int64.

@UnixJunkie

This comment has been minimized.

Show comment Hide comment
@UnixJunkie

UnixJunkie Jan 27, 2014

Member

By the way, Int64 are integers so no suffix seems fine.
Complex are pairs of floats so the dot suffix would seem a better hint to me
that I am not doing operations on simple integers.
But maybe some heavy user of complex numbers have a better opinion.

Member

UnixJunkie commented Jan 27, 2014

By the way, Int64 are integers so no suffix seems fine.
Complex are pairs of floats so the dot suffix would seem a better hint to me
that I am not doing operations on simple integers.
But maybe some heavy user of complex numbers have a better opinion.

@hcarty

This comment has been minimized.

Show comment Hide comment
@hcarty

hcarty Jan 27, 2014

Contributor

@UnixJunkie - changing to no suffix for Int64 and Int32 still leaves the ugly array indexing issue mentioned by @gasche in the Mantis link above.

Contributor

hcarty commented Jan 27, 2014

@UnixJunkie - changing to no suffix for Int64 and Int32 still leaves the ugly array indexing issue mentioned by @gasche in the Mantis link above.

@c-cube

This comment has been minimized.

Show comment Hide comment
@c-cube

c-cube Feb 4, 2014

Member

So. What's the conclusion here? Postfixed operators rather than shadowing (for the comparison operators such as < or >=)?

Member

c-cube commented Feb 4, 2014

So. What's the conclusion here? Postfixed operators rather than shadowing (for the comparison operators such as < or >=)?

@hcarty

This comment has been minimized.

Show comment Hide comment
@hcarty

hcarty Feb 5, 2014

Contributor

Please use suffixes everywhere or nowhere for consistency. My vote goes for the dot suffix.

Contributor

hcarty commented Feb 5, 2014

Please use suffixes everywhere or nowhere for consistency. My vote goes for the dot suffix.

@c-cube

This comment has been minimized.

Show comment Hide comment
@c-cube

c-cube Mar 2, 2014

Member

Hope that will reach a consensus! :)

Member

c-cube commented Mar 2, 2014

Hope that will reach a consensus! :)

@rgrinberg

This comment has been minimized.

Show comment Hide comment
@rgrinberg

rgrinberg Nov 16, 2014

Member

@hcarty are you satisfied with simon's changes? This was your feature request after all :D

Personally I don't particularly care about the names as long as they're consistent.

Member

rgrinberg commented Nov 16, 2014

@hcarty are you satisfied with simon's changes? This was your feature request after all :D

Personally I don't particularly care about the names as long as they're consistent.

@hcarty

This comment has been minimized.

Show comment Hide comment
@hcarty

hcarty Nov 17, 2014

Contributor

@rgrinberg I'm happy with these, thanks! And thanks to @c-cube as well.

Contributor

hcarty commented Nov 17, 2014

@rgrinberg I'm happy with these, thanks! And thanks to @c-cube as well.

rgrinberg added a commit that referenced this pull request Nov 17, 2014

@rgrinberg rgrinberg merged commit 43f419d into ocaml-batteries-team:master Nov 17, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment