Expose constructors for interval types

[LaurenceWarne]

Hi! I'd like to use intervals for which I know for example are only going to be of type [a, ∞), but using the constructor Interval.atOrAbove returns an Interval rather than an Above instance, so the ability to extract the lower bound as an exact ValueBound using x.lowerBound is lost AFAICS.

I think changing the return types would break binary compatibility, so this PR adds new methods on the companion which are in turn called by the corresponding methods on Interval, specifically for:

• Above
• Below
• All
• Empty

I think the constructors for these objects were leaked before, and included the internal flags value, and in general I'm not sure how much the Above, Below, etc are intended for general usage, but let me know any thoughts anyway 🙂

Thanks!

[armanbilge]

Thanks for the PR!

I think changing the return types would break binary compatibility

Actually I think we can do this without breaking compatibility. Instead of changing the return type, add a new method of the same name with the desired return type. Meanwhile, make the old method private and add a dummy type parameter to disambiguate e.g.

object Foo {
  // old
  private[Foo] def bar[DummyParameter](x: Int): AnyRef = ???
  // new
  def bar(x: Int): String = ???
}

[LaurenceWarne]

Actually I think we can do this without breaking compatibility. Instead of changing the return type, add a new method of the same name with the desired return type. Meanwhile, make the old method private and add a dummy type parameter to disambiguate e.g.

Nice! Though I find sbt mimaReportBinaryIssues still doesn't seem to like it: https://github.com/typelevel/spire/actions/runs/8364280484/job/22899223855?pr=1325#step:15:33?

[igstan]

I came across this PR by chance, but in general in the TypeLevel ecosystem, factories/smart constructors are designed to return the most generic type, so as to help type inference. That's why all those factories return Interval.

How about this solution instead. We expose factories on the companion objects of Above and Below, which allows us to hide the magic numbers and not break binary compatibility. The downside is that they may not be as accessible as the factories on Interval, but they're not that far away either.

Also, maybe the choice of names below isn't ideal and instead of inclusive/exclusive we could go with closed/open, respectively.

diff --git a/core/src/main/scala/spire/math/Interval.scala b/core/src/main/scala/spire/math/Interval.scala
index e142fb3b..9f2a609f 100644
--- a/core/src/main/scala/spire/math/Interval.scala
+++ b/core/src/main/scala/spire/math/Interval.scala
@@ -808,11 +808,21 @@ case class Above[A] private[spire] (lower: A, flags: Int) extends Interval[A] {
   def upperBound: Unbound[A] = Unbound()
 }
 
+object Above {
+  def inclusive[A](a: A): Above[A] = Above(a, Interval.closedLowerFlags)
+  def exclusive[A](a: A): Above[A] = Above(a, Interval.openLowerFlags)
+}
+
 case class Below[A] private[spire] (upper: A, flags: Int) extends Interval[A] {
   def lowerBound: Unbound[A] = Unbound()
   def upperBound: ValueBound[A] = if (isOpenUpper(flags)) Open(upper) else Closed(upper)
 }
 
+object Below {
+  def inclusive[A](a: A): Below[A] = Below(a, Interval.closedUpperFlags)
+  def exclusive[A](a: A): Below[A] = Below(a, Interval.openUpperFlags)
+}
+
 // Bounded, non-empty interval with lower < upper
 case class Bounded[A] private[spire] (lower: A, upper: A, flags: Int) extends Interval[A] {
   def lowerBound: ValueBound[A] = if (isOpenLower(flags)) Open(lower) else Closed(lower)
@@ -944,10 +954,10 @@ object Interval {
     if (lower < upper) Bounded(lower, upper, 1) else Interval.empty[A]
   def openUpper[A: Order](lower: A, upper: A): Interval[A] =
     if (lower < upper) Bounded(lower, upper, 2) else Interval.empty[A]
-  def above[A: Order](a: A): Interval[A] = Above(a, 1)
-  def below[A: Order](a: A): Interval[A] = Below(a, 2)
-  def atOrAbove[A: Order](a: A): Interval[A] = Above(a, 0)
-  def atOrBelow[A: Order](a: A): Interval[A] = Below(a, 0)
+  def above[A: Order](a: A): Interval[A] = Above.exclusive(a)
+  def below[A: Order](a: A): Interval[A] = Below.exclusive(a)
+  def atOrAbove[A: Order](a: A): Interval[A] = Above.inclusive(a)
+  def atOrBelow[A: Order](a: A): Interval[A] = Below.inclusive(a)
 
   private val NullRe = "^ *\\( *Ø *\\) *$".r
   private val SingleRe = "^ *\\[ *([^,]+) *\\] *$".r

[LaurenceWarne]

How about this solution instead. We expose factories on the companion objects of Above and Below, which allows us to hide the magic numbers and not break binary compatibility. The downside is that they may not be as accessible as the factories on Interval, but they're not that far away either.

Thanks for the input, that sounds similar to the approach taken in the PR initially: effe0b3, though I prefer your naming (+1 open/closed). I'll push up something in a bit 👍

Hopefully documentation will help with their discoverability.

[igstan]

@LaurenceWarne ah, yes, I missed that since I only looked at the final diff 👍🏻 BTW, I'm not a member/maintainer of this project, it was just a suggestion, so someone else will have to approve your changes.

[LaurenceWarne]

Fair point, I might lay off making changes until a maintainer appraisal - RE the other suggestion I'd need some guidance on the bin compat issues anyway 👍