Re: [containers] Proposal: Change to the Data.Map Monoid

This results in a silent change of semantics for all current users and _will_ break existing code _silently_. I am -1 on this change as there is no good way to manage the transition especially due to containers use as a boot package and the modicum of improvement strikes me as not worth the transition price. Moreover it is the "wrong" constraint. Maybe for instance is a terrible example to motivate this proposal as it adjoins a unit to a monoid pretending it is a semigroup to get a monoid. To union two maps we don't need a unit. Were Map/IntMap being defined from scratch? Reasonable. But given the very very large body of code that sits atop them that would break I have a hard time advocating for the ensuing debugging hell and silent breakages, Sent from my iPhone

On May 18, 2014, at 5:05 PM, Nick Partridge wrote:

Hi,

Currently the Monoid instance for Data.Map is implemented using union/unions, which are left biased. On key collision, it discards values from the right hand side of `mappend` - https://github.com/ghc/packages-containers/blob/bae098fb0a3994bc2b0ec3313004...

If you compare this with the Monoid for Maybe, it's like we're defaulting to First as the monoid instance for maps.

A more useful instance, however very much a breaking change, would be to make the instance depend on a Monoid (or better yet, a Semigroup) for the values in the map:

instance Monoid v => Monoid (Map k v) where mappend = unionWith mappend

This lets us build up maps with values in a useful Monoid, and mappend them with gusto.

Thoughts?

- Nick Partridge

Discussion period: 2 weeks. _______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries