Add LowerBounded and UpperBounded type classes

[izeigerman]

Implements #1536

[LukaJCB]

This is a great PR, thank you so much! I think we could add a bunch more instances, if you're up to it :)
We could also do it in a separate PR if you'd prefer? Or someone else could take them.
I'm thinking of

• Duration
• FiniteDuration
• various collections are lower bounded, e.g. List, Set, etc.
• Either, Validated, Ior, Option
• Tuple2
• Const

[LukaJCB]

I can also see another problem. What happens if someone summons both Order and Bounded, it seems we'd get a clash, probably, no?

def foo[A: Order: Bound](x: A, y: A) = x === y

The above will probably result in ambiguity.
We could go around this by using composition instead of inheritance:

trait LowerBound[A] {
  def lowerBound: A
  def partialOrder: PartialOrder[A]
}

[izeigerman]

@LukaJCB Thanks for pointing out the ambiguity issue!

I think we could add a bunch more instances, if you're up to it

Yes, I'm definitely up to it! I just wasn't quite sure about boundaries for some types that you enumerated. Can you please help me determine whether my understanding is correct:

• Duration - I assume only Lower Bound is applicable here and the value should be a duration with a 0 length. Upper Bound can't be specified since the duration can be infinite.
• FiniteDuration - Lower Bound - 0-length duration, Upper Bound - Long.MaxValue.
• List, Set, etc. - Lower Bound - empty collection. Upper Bound is not applicable.
• Either, Validated, Ior, Option - not sure about the semantics here. For example the Lower Bound for Option might be either None or the Lower Bound of the underlying type. Similar question about Either, Validated and Ior - what is the expected behavior here?
• Tuple2 - Lower Bound - is the Lower Bound for both types that constitute a sum. Same for the Upper Bound.
• Const - Lower and Upper bounds of the underlying constant type.

[codecov-io]

Codecov Report

Merging #2910 into master will increase coverage by 0.05%.
The diff coverage is 97.43%.

@@            Coverage Diff            @@
##           master   #2910      +/-   ##
=========================================
+ Coverage   94.24%   94.3%   +0.05%     
=========================================
  Files         363     366       +3     
  Lines        6951    7020      +69     
  Branches      184     180       -4     
=========================================
+ Hits         6551    6620      +69     
  Misses        400     400

Impacted Files	Coverage Δ
...e/src/main/scala-2.12-/cats/instances/stream.scala	97.26% <ø> (ø)	⬆️
core/src/main/scala/cats/instances/set.scala	100% <ø> (ø)	⬆️
core/src/main/scala/cats/instances/queue.scala	100% <ø> (ø)	⬆️
testkit/src/main/scala/cats/tests/CatsSuite.scala	33.33% <ø> (ø)	⬆️
core/src/main/scala/cats/instances/uuid.scala	100% <ø> (ø)	⬆️
core/src/main/scala/cats/instances/list.scala	100% <ø> (ø)	⬆️
core/src/main/scala/cats/instances/option.scala	100% <ø> (ø)	⬆️
core/src/main/scala/cats/instances/vector.scala	100% <ø> (ø)	⬆️
core/src/main/scala/cats/instances/either.scala	100% <ø> (ø)	⬆️
core/src/main/scala/cats/instances/tuple.scala	100% <ø> (ø)	⬆️
... and 27 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a0896a2...d8644c1. Read the comment docs.

[LukaJCB]

That seems inline with what I had in mind, except for:

Duration - I assume only Lower Bound is applicable here and the value should be a duration with a 0 length. Upper Bound can't be specified since the duration can be infinite.

There is Duration.Inf for the upper bound :)

Either, Validated, Ior, Option - not sure about the semantics here. For example the Lower Bound for Option might be either None or the Lower Bound of the underlying type. Similar question about Either, Validated and Ior - what is the expected behavior here?

We have to go with what the existing Order implementations assume, so for example for Either,
Left(x).compare(Right(y)) will always be -1. So Left is considered to be less than Right.

Does that make it more clear? :)

[izeigerman]

@LukaJCB Yes, makes perfect sense to me. Thanks!

[LukaJCB]

Btw, I don't see instances in kernel for Ior and Validated, so you can probably leave those out for now :)

[rossabaker]

Duration - I assume only Lower Bound is applicable here and the value should be a duration with a 0 length. Upper Bound can't be specified since the duration can be infinite.

Durations can be negative. See Duration.MinusInf.

[izeigerman]

Thanks @rossabaker !
@LukaJCB I have two more clarifications to make:

1. I noticed that List, Set, etc may actually have an upper bound according to their Order implementations. Basically if the underlying type A has an UpperBound instance then for example for List the UpperBound will look like:

def maxBound: List[A] = List(UpperBound[A].maxBound)

This still satisfies the law, so unless there are any objections I'm going to introduce those as well.
2. Making a partialOrder a member of the *Bounded seems to have a purely indicative/enforcing purpose. It's not being used in any other way. And of course it makes creating instances for this type class less convenient. At the same time I realize that the direct hierarchy is not an option. I just figured this worth pointing out.

UPD: Oops, sorry. My 1st point is completely wrong since I forgot about the scenario when the collection's size is > 1 and the first element equals to the upper bound. Please ignore it.

[LukaJCB]

Durations can be negative. See Duration.MinusInf.

TIL, that's kinda weird, though, what kind of duration is -4 minutes? 😄

[ritschwumm]

Durations can be negative. See Duration.MinusInf.

TIL, that's kinda weird, though, what kind of duration is -4 minutes? smile

imho a duration is just the difference between two points in time, which of course can be negative.

[djspiewak]

TIL, that's kinda weird, though, what kind of duration is -4 minutes? 😄

I mean, time is a directional vector, not a magnitude. I think -4.minutes is a pretty sane thing to talk about. :-) At the very very least, 6.minutes - 2.minutes is most definitely a sane thing to talk about, but that expression doesn't really make algebraic sense without inverses, which implies negative durations and thus a directionality to time.

[izeigerman]

@LukaJCB I think this PR is ready to be revisited once you get a chance. Thanks

[kailuowang]

Thanks so much for this contribution. I think we should avoid adding so many new bincompat traits in the master branch now we are very close to drop support for 2.11 on master. I.e. I am proposing separating this into two PRs,

1. a type class (with Const instance which can be added without bincompat trait) PR which can be merged right now,
2. an instances PR that simply added methods to existing traits, we can merge it after the master branch drops 2.11 support

[izeigerman]

@kailuowang I think the suggested approach sounds reasonable except one thing. I'm not sure I can just add an instance for Const without adding an instance for at least a single other type (eg Int), since I won't be able to test it properly. Do you think we should introduce instances for types that don't require bincompat traits (like Int, Short, etc.) as part of step #1? Or should we just skip tests for Const?

[kailuowang]

@izeigerman ah I didn't realize that there are instances you can add without bincompat traits. Yes please make them as part of step 1. Thanks!!!

[izeigerman]

The PR that covers step 1 is #2913