Rename NonEmpty.groupWith/sortWIth to *On*

8 views
Skip to first unread message

Philip Hazelden

unread,
Feb 12, 2020, 12:58:54 PM2/12/20
to core-librari...@haskell.org, libr...@haskell.org
Hi,

I'm referring to these functions: `sortWith` `groupWith` `groupAllWith` `groupWith1` `groupAllWith1`.

The `On` suffix is more common for such things, and I anticipate it's what most users will normally expect them to be named.

Additionally, the name `groupWith` is potentially misleading. That name is also used in GHC.Exts[1], for a function with the same type signature but different semantics. `GHC.Exts.groupWith` sorts its input list, unlike `NonEmpty.groupWith` or `Data.List.Extra.groupOn`. (In NonEmpty, we have `groupAllWith` for group-with-sort. So we have three names, two semantics, and no consistency.)

According to https://github.com/ekmett/semigroups/pull/52, `With` was chosen because:

> The preferred vocabulary for On here is With to match the combinators in GHC.Exts from the "comprehensive comprehensions" paper. That'd make it so that if you use these with comprehensive comprehensions turned on and RebindableSyntax turned on then you'd get these combinators.

But I don't see anything in the docs which suggests that the TransformListComp extension[2] uses these functions implicitly, or interacts with RebindableSyntax[3]. So perhaps I'm missing something, but my guess is this is no longer a concern.

The case for `sortWith` is weaker, since it might trip people up but it won't introduce unexpected semantics. There's also a counter argument in https://gitlab.haskell.org/ghc/ghc/issues/12044. `Data.List.sortOn` only computes the mapping function once for each element, while `GHC.Exts.sortWith` computes it every time it does a comparison but uses fewer allocations. `Data.NonEmpty.sortWith` acts like the latter. My suggestion would be to replace it with a `sortOn` implemented like `Data.List.sortOn`, but I also don't think it would be terrible to have both, or even to just rename and leave this small inconsistency. If there's no agreement on this function, I think it would be worth renaming the group functions anyway.

And so I propose, in descending order of how much I care:

1. Rename the group functions to be named "On" instead of "With".

2. Rename `sortWith` to `sortOn` In the case of `sortWith`, also reimplement it as

3. Reimplement `sortOn` as

    sortOn :: Ord b => (a -> b) -> NonEmpty a -> NonEmpty a
    sortOn = fmap snd . sortBy (comparing fst) . fmap (\x -> let y = f x in y `seq` (y, x))

I assume the process here would be to leave the existing names for a time with deprecation warnings, and then remove them a few versions down the line.


Best,
Phil
Reply all
Reply to author
Forward
0 new messages