Rename NonEmpty.groupWith/sortWith functions to *On*
Motivation
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, 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 uses these functions implicitly, or interacts with RebindableSyntax. So 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 #12044 (closed). 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. If there's no agreement on this function, I think it would be worth renaming the group functions anyway.
Proposal
Rename these functions to have "On" instead of "With". In the case of sortWith
, also reimplement it 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.