purescript-arrays icon indicating copy to clipboard operation
purescript-arrays copied to clipboard

`groupBy` has unexpected `l` argument for `a -> a -> Boolean` predicate

Open JordanMartinez opened this issue 2 years ago • 3 comments

See https://try.purescript.org/?gist=26968458c1261226b33e574128ba9aae

For a given array like [0, 1, 2], I expect the l and r arguments to be

  • first run: 0 and 1
  • second run: 1 and 2

However, in the second run, l is 0 rather than 1.

I'd argue that the behavior here is unexpected.

JordanMartinez avatar Aug 08 '22 17:08 JordanMartinez

This comes down to the definition of "equivalence relation". (\a b -> a + 1 == b) is a useful equivalence relation, it's just not what the implementation expects. It would be nice if this worked as grouping consecutive elements is a fairly common thing to do.

natefaubion avatar Aug 08 '22 17:08 natefaubion

(\a b -> a + 1 == b) is not an equivalence relation, as it is not reflexive, symmetric, or transitive. Obviously the closure of that relation would be, but that would not be useful

It may be a useful relation, but groupBy does specify an equivalence relation.

(To be clear the proposed change to operate only on successive pairs does not change behaviour if the relation is an equivalence relation and I'm happy with that change in principle)

nwolverson avatar Oct 18 '22 19:10 nwolverson

To be clear the proposed change to operate only on successive pairs does not change behaviour if the relation is an equivalence relation and I'm happy with that change in principle

So, should the PR I have open for this #230 be accepted? Or should a new function be defined that has #230's implementation?

JordanMartinez avatar Oct 27 '22 12:10 JordanMartinez