Nick Partridge | 19 May 02:05 2014
Picon

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

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/bae098fb0a3994bc2b0ec3313004b40cd097ed8d/Data/Map/Base.hs#L341-L344

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 <at> haskell.org
http://www.haskell.org/mailman/listinfo/libraries
Edward Kmett | 19 May 04:19 2014
Picon

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 <nkpart <at> gmail.com> 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/bae098fb0a3994bc2b0ec3313004b40cd097ed8d/Data/Map/Base.hs#L341-L344

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 <at> haskell.org
http://www.haskell.org/mailman/listinfo/libraries
_______________________________________________
Libraries mailing list
Libraries <at> haskell.org
http://www.haskell.org/mailman/listinfo/libraries
Alexander Berntsen | 19 May 09:40 2014
Picon

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


On 19/05/14 04:19, Edward Kmett wrote:
> This results in a silent change of semantics for all current users 
> and _will_ break existing code _silently_.
-1 on account of this.
--

-- 
Alexander
alexander <at> plaimi.net
https://secure.plaimi.net/~alexander
Johan Tibell | 19 May 12:04 2014
Picon

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

-1 for the reasons Edward gave.
_______________________________________________
Libraries mailing list
Libraries <at> haskell.org
http://www.haskell.org/mailman/listinfo/libraries
Henning Thielemann | 19 May 07:05 2014
Picon

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

Am 19.05.2014 02:05, schrieb Nick Partridge:

> 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.

This was discussed two years ago:
   http://www.haskell.org/pipermail/libraries/2012-April/017743.html

That said, I'd also prefer your instance.

Gmane