feat: Add Range module (#7128)

[LionelMeli]

No description provided.

[magnus-madsen]

Hi @LionelMeli

Thanks for your contribution. We are very busy with some big merges, but I promise we will get around to reviewing it.

[magnus-madsen]

Perhaps @stephentetley could take a look, since he has experience with Haskell? Otherwise perhaps @mlutze or @JonathanStarup

[stephentetley]

I can take a look as I'm on holiday today.

One thing I noticed first I'd change order of the BoundType constructors so Exclusive < Inclusive holds as Inclusive will make a larger range.

[LionelMeli]

Hi all
No urgency here!
Feel free to review after the high level priority issues.

[stephentetley]

Hi Lionel - here are my thoughts...

There are (at least) two range libraries on Hackage - I looked at the other one first - so I'd change the comment to acknowledge the particular one [1] and probably its author Robert Massaioli.

I'm not so keen on the operators - initially I thought they were versions of the vector-space operators until I looked closely to see what they were doing. Once associated types are production ready in Flix it would be nice to have the vector-space / affine-space operators [2] in the standard library as they are very general, so I think we should wait for them to pick operator symbols.

I'd propose the following named functions as replacements:

+=+ => inclFromTo
+=* => inclFromExclTo
*=+ => exclFromInclTo
*=* => exclFromTo

I didn't see the infinity shorthand constructors in Flix that are in the Haskell library so these could be added if you want to (I'm not sure about the last two):

lbi => inclToInf
lbe => exclToInf

ubi => infToIncl ??
ube => infToExcl ??

It would be nice if zeroTo etc. were polymorphic in the number type but that could be done later as an improvement.

[1] https://hackage.haskell.org/package/range
[2] https://hackage.haskell.org/package/vector-space

[LionelMeli]

Hi Stephen,

I'm not implemented the lbi, ube etc operators because it's my next step... (Sorry I didn't explain in the PR).
For the moment I just delivered (and focused) about this set of features. I take care about your comments.
By the way I plan to extend this module with other operators you mentioned and also a set of functions like aboveRange etc. (next step)

I don't know the library https://hackage.haskell.org/package/vector-space (At a glance maybe a little bit difficult for me as I'm not fuent in Haskell and Functional Programming!). No problem, It'll be just an another next step!

PS: What do you think about this kind of naming (from: https://github.com/google/guava/wiki/RangesExplained) ?

Range type	Method
(a..b)	open(C, C)
[a..b]	closed(C, C)
[a..b)	closedOpen(C, C)
(a..b]	openClosed(C, C)
(a..+∞)	greaterThan(C)
[a..+∞)	atLeast(C)
(-∞..b)	lessThan(C)
(-∞..b]	atMost(C)
(-∞..+∞)	all()

[stephentetley]

Thanks Lionel

Vector Space is quite different to Range - but it adds complementary "maths" operators to the common ones that we already have in the standard library, it would be nice if there are operators available for it (if we want the classes from it in the standard library of course).

I think your proposed function names are very good - definitely better than mine. The Flix stdlib doesn't generally have abbreviated names so avoiding then is a winner, I only chose to use abbreviations to avoid long names as I was suggesting them as alternatives to operators.

[LionelMeli]

Thanks for your answers.

[magnus-madsen]

@LionelMeli Why did you close this? Are we not iterating towards some implementation? :)

[LionelMeli]

Hello Magnus,

the only reason is: I did wrong git commands... I reopen this one once I push the code again! Sorry for this.

[magnus-madsen]

Okay, no problem!

[magnus-madsen]

@LionelMeli @stephentetley Hi guys. Where are we with this?

I would like to have a Data.Range which can be used to iterate through arrays and vector indices.

In particular, without knowing the specifics, it would be nice if we can have a function:

mod Array {
  pub def indices(a: Array[t, r]): Range[Int32]
}

or something like this where Range is then iterable so we can use it in a foreach loop.

[magnus-madsen]

Looking at the code (briefly)

• The definitions of BoundType and Bound looks sensible, but they should probably be in their own files and modules.
• The definition of Range looks reasonable, if perhaps a bit over-engineered?
• What is this IteratorFactory? (I once promised myself that Flix would not have "factories" :))
• All public functions, e.g. open, closed, etc. should have docs.
• I would want to implement Iterable directly on Range, but it is unclear if I can do so?
• Perhaps we should leave out to fancy functor and map stuff. (It can come later in a subsequent PR).
• Enumerable looks like its own abstraction. Perhaps we should start by adding that in its own PR?

[magnus-madsen]

Addition: It would best if iteration through the array indices does not require allocation.

[magnus-madsen]

I was looking at:

https://typelevel.org/cats-collections/range.html

and

https://doc.rust-lang.org/std/ops/struct.Range.html

which seems to have a simpler data type.

What's the upshot of the more complex Haskell one? That is clearer?

[LionelMeli]

Hi @magnus-madsen
Thanks for your review.
Let's see in details your remarks in next weeks.

[LionelMeli]

Hi @magnus-madsen

Looking at the code (briefly)

• The definitions of BoundType and Bound looks sensible, but they should probably be in their own files and modules.
  Ok (however if we choose basic Range type we don't have these types anymore)
• The definition of Range looks reasonable, if perhaps a bit over-engineered?
  You are right. I heavily follow the Data.Range from Haskell as it little bit more complicated compare to https://typelevel.org/cats-collections/range.html. I've a question: Do you only prefer inclusive Range (i.e. [a,b]) or a mix inclusive and exclusive bounds (e.g. [a,b[ or ]a,b[ ) ?
• What is this IteratorFactory? (I once promised myself that Flix would not have "factories" :))
  The IteratorFactory is due to in Flix 0.45.0 we don't have "Associated Type" in Traits. As I know @stephentetley added (cf. refactor: add assoc type and effect to Iterable (part of issue #7433) #7615 ) this feature. So I can remove this one (I'll work with the night build as the Flix 0.46.0 not include yet this Stephen's refactoring)
• All public functions, e.g. open, closed, etc. should have docs.
  Ok
• I would want to implement Iterable directly on Range, but it is unclear if I can do so?
  Yes, I think we can now thanks to "Associated Type"
• Perhaps we should leave out to fancy functor and map stuff. (It can come later in a subsequent PR).
  Sure
• Enumerable looks like its own abstraction. Perhaps we should start by adding that in its own PR?
  Sure (but as I refactor the code - and simplified this one - Enumerable should be vanished...)

[LionelMeli]

Reopened with "Right" commits!