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

proposal: add package 'operators' #58559

Closed
mrwonko opened this issue Feb 16, 2023 · 34 comments
Closed

proposal: add package 'operators' #58559

mrwonko opened this issue Feb 16, 2023 · 34 comments
Labels
Milestone

Comments

@mrwonko
Copy link

mrwonko commented Feb 16, 2023

When using the github.com/google/go-cmp/cmp/cmpopts package, I sometimes find myself writing code like

cmpopts.SortSlices(func(a, b string) bool { return a < b })

Now that we have generics, there could be an operators package like in Python that provides the operators as functions, like

func Lt[T constraints.Ordered](lhs, rhs T) bool {
	return lhs < rhs
}

so that I can write

cmpopts.SortSlices(operators.Lt[string])

instead. Or maybe it should be LT, given Go capitalises initialisms.

@gopherbot gopherbot added this to the Proposal milestone Feb 16, 2023
@seankhliao

This comment was marked as outdated.

@seankhliao seankhliao marked this as a duplicate of #27605 Feb 16, 2023
@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Feb 16, 2023
@mrwonko

This comment was marked as resolved.

@seankhliao seankhliao reopened this Feb 16, 2023
@seankhliao seankhliao marked this as not a duplicate of #27605 Feb 16, 2023
@seankhliao
Copy link
Member

see also #21498

@DeedleFake
Copy link

DeedleFake commented Feb 16, 2023

While a package like this could certainly be useful, I think a more general solution, like lightweight functions, would be better overall.

@golightlyb
Copy link
Contributor

golightlyb commented Feb 17, 2023

One generic implementation here:
https://pkg.go.dev/github.com/tawesoft/golib/v2/operator

e.g. cmpopts.SortSlices(operator.LT[string])

@mrwonko
Copy link
Author

mrwonko commented Feb 17, 2023

A quick way to turn a generic operator into a concrete function would address this better, agreed. But that requires a language-level change, while this is a library-only solution. But I'd be happy with either.

@earthboundkid
Copy link
Contributor

I dismissed this proposal at first because I was distracted by the usecase which doesn't strike me as very compelling, but on further reflection, this is a great idea for what to do about math/v2. I think the proposal should be to add a new package called math/ops which has Min, Max, Abs, Compare(T) int and the operators: Eq, Gt, Gte, etc. Instead of using constraints.Ordered, it should define its own constraint, ops.Orderable and the constraints package should be quietly abandoned, see #52427.

@earthboundkid
Copy link
Contributor

Can the title of the issue be changed to "proposal: add package math/ops" or to use mrwonko's original package name "proposal: add package operators"? The current title is a bit misleading.

@ianlancetaylor ianlancetaylor changed the title proposal: operators: operators as functions proposal: add package operators Feb 21, 2023
@golightlyb
Copy link
Contributor

golightlyb commented Feb 23, 2023

I dismissed this proposal at first because I was distracted by the usecase which doesn't strike me as very compelling, but on further reflection, this is a great idea for what to do about math/v2. I think the proposal should be to add a new package called math/ops which has Min, Max, Abs, Compare(T) int and the operators: Eq, Gt, Gte, etc. Instead of using constraints.Ordered, it should define its own constraint, ops.Orderable and the constraints package should be quietly abandoned, see #52427.

Strings can be compared GT, LTE etc. too but i wouldn't call that maths.

@earthboundkid
Copy link
Contributor

How about package sort/ops?

@golightlyb
Copy link
Contributor

How about package sort/ops?

Makes sense. Or even just the sort package itself.

@earthboundkid
Copy link
Contributor

earthboundkid commented Feb 23, 2023

Python's operators package has add, mul, mod, etc. I don't think those really make sense for Go. How about this:

package cmp // import "sort/cmp"

func Eq[T comparable](a, b T) bool
func Ne[T comparable](a, b T) bool

type Orderable interface { /* ... */ }

func Gt[T Orderable](a, b T) bool
func Gte[T Orderable](a, b T) bool
func Lt[T Orderable](a, b T) bool
func Lte[T Orderable](a, b T) bool

func Compare[T Orderable](a, b T) int

func Min[T Orderable](a, b T) T
func Max[T Orderable](a, b T) T
func Abs[T Orderable](t T) T

Any objections? Additions? Note that Compare doesn't really work for floats because of NaN, so you should use Go 1.21's math.Compare instead.

Edited.

@DeedleFake
Copy link

DeedleFake commented Feb 23, 2023

@carlmjohnson What do the comparison functions compare to? You've got, for example, func Eq[T comparable](other T) bool. What does other get compared against? Edit: 'twas fixed. Never mind.

Which makes me wonder: As Go doesn't have partial application, would a second set of functions that return another function make sense? Something like func EqTo[T comparable](a T) func(b T) bool such that Eq(1)(1) is true?

@earthboundkid
Copy link
Contributor

D'oh, yes, that's a thinko. I somehow got thinking about it as a method. Will edit the list.

@mrwonko
Copy link
Author

mrwonko commented Feb 24, 2023

As Go doesn't have partial application, would a second set of functions that return another function make sense?

That feels inelegant, a special case solution to a general problem. Maybe lightweight anonymous functions (#21498) could be a better solution for that? Or some kind of func Partial2[T any, U any, V any](func(T, U) V, T) func(U) V helper in a separate package, although without variadic generics, you end up needing one of those per arity.

@DeedleFake
Copy link

DeedleFake commented Feb 24, 2023

Note that there is a proposal for variadic generics: #56462. Partial() feels kind of weird to me, though. Maybe it's just the name. Hmmm... slices.ContainsFunc(s, Partial(cmp.Eq, 3)). Yeah, it feels weird to me compared to slices.ContainsFunc(s, cmp.EqTo(3)).

@earthboundkid
Copy link
Contributor

How about this then:

package cmp // import "sort/cmp"

func Eq[T comparable](a, b T) bool
func Ne[T comparable](a, b T) bool

func EqTo[T comparable](T) func(T) bool
func NeTo[T comparable](T) func(T) bool

type Orderable interface { /* ... */ }

func Gt[T Orderable](a, b T) bool
func Gte[T Orderable](a, b T) bool
func Lt[T Orderable](a, b T) bool
func Lte[T Orderable](a, b T) bool

func GtOf[T Orderable](T) func(T) bool  // GtTo isn't grammatical, so Of?
func GteTo[T Orderable](T) func(T) bool
func LtOf[T Orderable](T) func(T) bool
func LteTo[T Orderable](T) func(T) bool

func Compare[T Orderable](a, b T) int

func Min[T Orderable](val, vals ...T) T
func Max[T Orderable](va, vals ...T) T
func Clamp[T Orderable](min, val, max T) T  // Should add this too
func Abs[T Orderable](t T) T

@DeedleFake
Copy link

DeedleFake commented Feb 24, 2023

func Min(val, vals ...T) won't compile. I also feel like func Min(val T, vals ...T) T might tie it to variadic use too much. You would be able to do Min(1, 2, 3), but if you already had a slice you'd have to do Min(s[0], s[1:]...), which is weird, though I suppose that it would avoid the case of figuring out what to return if given no arguments or an empty slice.

Edit: greater than of doesn't work any more than greater than to does, unfortunately. I don't think there's any short thing that could be added that would really work there, unfortunately. a and b are equal vs. a is equal to... works for Eq() and EqTo(), I think, but a is greater than b is already directional, so...

@mrwonko
Copy link
Author

mrwonko commented Feb 24, 2023

Partial() feels kind of weird to me, though. Maybe it's just the name.

Another popular name for that kind of function is Bind(), as seen e.g. in C++.

@golightlyb
Copy link
Contributor

These to/of functions look reqlly ugly to me and I'm not sure there's a much need compared to the plain operators commonly used for sorting. There are other uses for operators as functions but sorting is the major one.

We already have Contains in /x/slices, so no need for EqTo in a ContainsFunc. What would you want these partially applied functions for that its worth them being added to the stdlib?

@DeedleFake
Copy link

Bind() sounds a lot better to me. JavaScript uses bind(), too, though it's as a method.

@earthboundkid
Copy link
Contributor

These to/of functions look reqlly ugly to me and I'm not sure there's a much need compared to the plain operators commonly used for sorting. There are other uses for operators as functions but sorting is the major one.

Yeah. I think the main use is to just pass op.Gt to slices.SortFunc. My guess is that, not counting min/max/clamp, Gt will get the most use out of anything in the package, then maybe Compare, followed by everything else in the long tail.

@earthboundkid
Copy link
Contributor

earthboundkid commented Feb 24, 2023

New try:

package cmp // import "sort/cmp"

func Eq[T comparable](a, b T) bool
func Ne[T comparable](a, b T) bool

type Orderable interface { /* ... */ }

func Gt[T Orderable](a, b T) bool
func Gte[T Orderable](a, b T) bool
func Lt[T Orderable](a, b T) bool
func Lte[T Orderable](a, b T) bool

type Type[T Orderable] struct{ Val T }

// For type inference
func Op[T Orderable](t T) Type[T] { return Type[T]{t} }

func (Type[T]) Eq(other T) bool
func (Type[T]) Ne(other T) bool
func (Type[T]) Gt(other T) bool
func (Type[T]) Gte(other T) bool
func (Type[T]) Lt(other T) bool
func (Type[T]) Lte(other T) bool
func (Type[T]) Compare(other T) int

func Compare[T Orderable](a, b T) int

func Min[T Orderable](val T, vals ...T) T // fixed typo
func Max[T Orderable](val T, vals ...T) T
func Clamp[T Orderable](min, val, max T) T
func Abs[T Orderable](t T) T

In another package slices.BinarySearchFunc(s, cmp.Op(1).Gte).

@DeedleFake
Copy link

DeedleFake commented Feb 24, 2023

Introducing a type in between is an interesting option. If it wasn't for the fact that it's a wrapper, method expressions and method values would allow that single method to function in either way:

partial := Op(a).Lt // func(T) bool
full := Op[T].Lt // func(Type[T], T) bool

@gophun
Copy link

gophun commented Feb 25, 2023

This proposal seems to be motivated by a desire for saving a couple of keystrokes, because seeing the actual operator literally on the page is clearer than hiding it behind a cryptic name.

cmpopts.SortSlices(func(a, b string) bool { return a < b })

cmpopts.SortSlices(operators.Lt[string])

@jarrodhroberson
Copy link

This reeks of worst kitchen sink Java/Python bloat. Support for defining proper definitions for operators on structs as user definable functions would be much better and help solve the comparable problem with struct we have now.

@earthboundkid
Copy link
Contributor

#59488 has basically obsoleted this proposal. You could try to get more functions added to cmp though.

@rsc
Copy link
Contributor

rsc commented Jul 12, 2023

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@willfaught
Copy link
Contributor

Support for defining proper definitions for operators on structs as user definable functions would be much better and help solve the comparable problem with struct we have now.

Agreed. Built-in types could have methods for these things, that operations are syntactic sugar for, at least conceptually. int16 could have an Equals(int16) bool method. Generic constraints could describe these methods where appropriate.

@ianlancetaylor
Copy link
Contributor

We have several times considered and rejected defining methods for built-in types. That is not this proposal. If you want to discuss that, please do it on a forum or in a different issue. Thanks.

@ianlancetaylor
Copy link
Contributor

The stated reason for this proposal is to have comparison functions for sorting purposes. As @carlmjohnson points out, this has been done in the upcoming 1.21 release with the new cmp.Less and cmp.Compare functions. Is there any other strong reason to adopt this proposal? Thanks.

@adonovan adonovan changed the title proposal: add package operators proposal: add package 'operators' Jul 19, 2023
@rsc
Copy link
Contributor

rsc commented Jul 26, 2023

To repeat what Ian said two weeks ago, we've standardized on cmp.Compare for comparison, not less-than functions. It sounds like cmpopts should add a new function that takes a 3-way compare function.

@rsc
Copy link
Contributor

rsc commented Jul 26, 2023

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Aug 2, 2023

No change in consensus, so declined.
— rsc for the proposal review group

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Declined
Development

No branches or pull requests