Add LowerBounded and UpperBounded type classes #2910

izeigerman · 2019-06-24T22:39:59Z

Implements #1536

LukaJCB · 2019-06-24T23:04:50Z

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 · 2019-06-24T23:08:40Z

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 · 2019-06-24T23:24:05Z

@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 · 2019-06-24T23:29:48Z

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 · 2019-06-24T23:35:04Z

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 · 2019-06-24T23:36:25Z

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

LukaJCB · 2019-06-24T23:37:28Z

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

rossabaker · 2019-06-25T03:02:18Z

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 · 2019-06-25T03:31:51Z

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

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 · 2019-06-25T13:06:56Z

Durations can be negative. See Duration.MinusInf.

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

ritschwumm · 2019-06-25T13:55:18Z

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 · 2019-06-25T16:16:47Z

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 · 2019-06-26T14:56:46Z

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

kailuowang · 2019-06-26T15:17:18Z

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,

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

izeigerman · 2019-06-26T16:31:55Z

@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 · 2019-06-26T16:37:10Z

@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 · 2019-06-26T20:01:37Z

The PR that covers step 1 is #2913

izeigerman added 2 commits June 24, 2019 15:02

Add LowerBounded, UpperBounded and Bounded type classes

81d363a

Fix formatting

db1da8a

Add more instances

9ac265b

izeigerman added 2 commits June 25, 2019 15:21

Address binary incompatibility

69198ae

Fix 2.13 build

d8644c1

izeigerman changed the title ~~Add LowerBounded, UpperBounded and Bounded type classes~~ Add LowerBounded and UpperBounded type classes Jun 25, 2019

izeigerman mentioned this pull request Jun 26, 2019

Add LowerBounded and UpperBounded typeclasses #2913

Merged

izeigerman closed this Jun 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add LowerBounded and UpperBounded type classes #2910

Add LowerBounded and UpperBounded type classes #2910

izeigerman commented Jun 24, 2019

LukaJCB commented Jun 24, 2019 •

edited

Loading

LukaJCB commented Jun 24, 2019

izeigerman commented Jun 24, 2019

codecov-io commented Jun 24, 2019 •

edited

Loading

LukaJCB commented Jun 24, 2019 •

edited

Loading

izeigerman commented Jun 24, 2019

LukaJCB commented Jun 24, 2019

rossabaker commented Jun 25, 2019

izeigerman commented Jun 25, 2019 •

edited

Loading

LukaJCB commented Jun 25, 2019

ritschwumm commented Jun 25, 2019

djspiewak commented Jun 25, 2019

izeigerman commented Jun 26, 2019

kailuowang commented Jun 26, 2019

izeigerman commented Jun 26, 2019 •

edited

Loading

kailuowang commented Jun 26, 2019

izeigerman commented Jun 26, 2019

Add LowerBounded and UpperBounded type classes #2910

Add LowerBounded and UpperBounded type classes #2910

Conversation

izeigerman commented Jun 24, 2019

LukaJCB commented Jun 24, 2019 • edited Loading

LukaJCB commented Jun 24, 2019

izeigerman commented Jun 24, 2019

codecov-io commented Jun 24, 2019 • edited Loading

Codecov Report

LukaJCB commented Jun 24, 2019 • edited Loading

izeigerman commented Jun 24, 2019

LukaJCB commented Jun 24, 2019

rossabaker commented Jun 25, 2019

izeigerman commented Jun 25, 2019 • edited Loading

LukaJCB commented Jun 25, 2019

ritschwumm commented Jun 25, 2019

djspiewak commented Jun 25, 2019

izeigerman commented Jun 26, 2019

kailuowang commented Jun 26, 2019

izeigerman commented Jun 26, 2019 • edited Loading

kailuowang commented Jun 26, 2019

izeigerman commented Jun 26, 2019

LukaJCB commented Jun 24, 2019 •

edited

Loading

codecov-io commented Jun 24, 2019 •

edited

Loading

LukaJCB commented Jun 24, 2019 •

edited

Loading

izeigerman commented Jun 25, 2019 •

edited

Loading

izeigerman commented Jun 26, 2019 •

edited

Loading