Add Foldable#[max|min]ByOption, Reducible#[maxBy|minBy]

[kubukoz]

No description provided.

[leusgalvan]

Can I take this?

[LukaJCB]

@leusgalvan go right ahead!
You'll have to add these to a separate syntax trait, due to bin compatibility, just a heads up :)
Feel free to ask questions, about that!

[leusgalvan]

@LukaJCB sorry for the delay. Would love to know a little bit more about how bin compatibility is managed in Cats, is there any source you can point me to? In particular I'd like to know the relationship between the *Syntax, *Ops and *Compat classes.

From what I've read in the contributing guide I can use mimaReportBinaryIssues to detect the issues.

Thanks in advance!

[LukaJCB]

The *Ops classes are classes that add direct syntax to certain type classes.
A *Syntax class has several implicit conversions from a type to their corresponding *Ops class, to provide the given syntax.
*Compat classes are the same as *Syntax classes, but are named differently because they're only needed because we can't put anything new inside existing *Syntax classes without breaking binary compatability.

With that said, I actually think you can drop them right here in this FoldableOps class:
https://github.com/typelevel/cats/blob/master/core/src/main/scala/cats/syntax/foldable.scala#L116

You could look at this PR for inspiration: #2366 :)

[leusgalvan]

I've had more time to work on this and I'm almost finished adding the implementations to Reducible, Foldable and UnorderedFoldable.

I added the [max/min]ByOption to both Foldable and UnorderedFoldable through syntax. I provided the overriden implementation in Foldable because there I can use reduceLeftOption not available in UnorderedFoldable.

However, for cases where a type is both Foldable and UnorderedFoldable (e.g. List) the [max/min]ByOption methods are ambigous (they can be reached through either of the conversions).

What is the best way to handle this? I don't like the solutions I can think of: remove the implementation from Foldable or rename the methods in Unfoldable with unordered[max/min]ByOption.

[joroKr21]

@leusgalvan I found myself needing this very often. I mean one can just pass an Order.by explicitly but it's a thorn in my eye. Idk if you're still working on this but how about adding them just to Foldable for the time being? Otherwise it would be inconsistent to have minimumByOption but no minimumOption in UnorderedFoldable. We can then have a separate issue for moving those up to UnorderedFoldable in the next breaking release.