Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

Next Topic
 
classic Classic list List threaded Threaded
37 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

David Feuer
Many people have recognized for years that the Semigroup and Monoid
instances for Data.Map, Data.IntMap, and Data.HashMap are not so
great. In particular, when the same key is present in both maps, they
simply use the value from the first argument, ignoring the other one.
This somewhat counter-intuitive behavior can lead to bugs. See, for
example, the discussion in Tim Humphries's blog post[*]. I would like
do do the following:

1. Deprecate the Semigroup and Monoid instances for Data.Map.Map,
Data.IntMap.IntMap, and Data.HashMap.HashMap in the next releases of
containers and unordered-containers.

2. Remove the deprecated instances.

3. After another several years (four or five, perhaps?), make a major
release of each package in which the instances are replaced with the
following:

  instance (Ord k, Semigroup v) => Semigroup (Map k v) where
    (<>) = Data.Map.Strict.unionWith (<>)
  instance (Ord k, Semigroup v) => Monoid (Map k v) where
    mempty = Data.Map.Strict.empty

  instance Semigroup v => Semigroup (IntMap v) where
    (<>) = Data.IntMap.Strict.unionWith (<>)
  instance Semigroup v => Monoid (IntMap v) where
    mempty = Data.IntMap.Strict.empty

  instance (Eq k, Hashable k, Semigroup v) => Semigroup (HashMap k v) where
    (<>) = Data.HashMap.Strict.unionWith (<>)
  instance (Eq k, Hashable k, Semigroup v) => Monoid(HashMap k v) where
    mempty = Data.HashMap.Strict.empty

Why do I want the strict versions? That choice may seem a bit
surprising, since the data structures are lazy. But the lazy versions
really like to leak memory, making them unsuitable for most practical
purposes.

The big risk:

Someone using old code or old tutorial documentation could get subtly
wrong behavior without noticing. That is why I have specified an
extended period between removing the current instances and adding the
desired ones.

Alternatives:

1. Remove the instances but don't add the new ones. I fear this may
lead others to write their own orphan instances, which may not even
all do the same thing.

2. Write separate modules with newtype-wrapped versions of the data
structures implementing the desired instances. Unfortunately, this
would be annoying to maintain, and also annoying to use--packages
using the unwrapped and wrapped versions will use different types.
Manually wrapping and unwrapping to make the different types work with
each other will introduce lots of potential for mistakes and
confusion.

Discussion period: three weeks.

[*] http://teh.id.au/posts/2017/03/03/map-merge/index.html
_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

Andrew Martin
I am strongly in favor of this proposal. I've actually proposal the same thing (with additional motivations) on the GHC Trac: https://ghc.haskell.org/trac/ghc/ticket/1460

I also agree the Monoid instance should be strict in the values. I'm not sure that the Monoidless Map phase needs to last four or five years (I was thinking more like two or three years), but that discussion could happen later.

On Tue, Feb 13, 2018 at 2:33 PM, David Feuer <[hidden email]> wrote:
Many people have recognized for years that the Semigroup and Monoid
instances for Data.Map, Data.IntMap, and Data.HashMap are not so
great. In particular, when the same key is present in both maps, they
simply use the value from the first argument, ignoring the other one.
This somewhat counter-intuitive behavior can lead to bugs. See, for
example, the discussion in Tim Humphries's blog post[*]. I would like
do do the following:

1. Deprecate the Semigroup and Monoid instances for Data.Map.Map,
Data.IntMap.IntMap, and Data.HashMap.HashMap in the next releases of
containers and unordered-containers.

2. Remove the deprecated instances.

3. After another several years (four or five, perhaps?), make a major
release of each package in which the instances are replaced with the
following:

  instance (Ord k, Semigroup v) => Semigroup (Map k v) where
    (<>) = Data.Map.Strict.unionWith (<>)
  instance (Ord k, Semigroup v) => Monoid (Map k v) where
    mempty = Data.Map.Strict.empty

  instance Semigroup v => Semigroup (IntMap v) where
    (<>) = Data.IntMap.Strict.unionWith (<>)
  instance Semigroup v => Monoid (IntMap v) where
    mempty = Data.IntMap.Strict.empty

  instance (Eq k, Hashable k, Semigroup v) => Semigroup (HashMap k v) where
    (<>) = Data.HashMap.Strict.unionWith (<>)
  instance (Eq k, Hashable k, Semigroup v) => Monoid(HashMap k v) where
    mempty = Data.HashMap.Strict.empty

Why do I want the strict versions? That choice may seem a bit
surprising, since the data structures are lazy. But the lazy versions
really like to leak memory, making them unsuitable for most practical
purposes.

The big risk:

Someone using old code or old tutorial documentation could get subtly
wrong behavior without noticing. That is why I have specified an
extended period between removing the current instances and adding the
desired ones.

Alternatives:

1. Remove the instances but don't add the new ones. I fear this may
lead others to write their own orphan instances, which may not even
all do the same thing.

2. Write separate modules with newtype-wrapped versions of the data
structures implementing the desired instances. Unfortunately, this
would be annoying to maintain, and also annoying to use--packages
using the unwrapped and wrapped versions will use different types.
Manually wrapping and unwrapping to make the different types work with
each other will introduce lots of potential for mistakes and
confusion.

Discussion period: three weeks.

[*] http://teh.id.au/posts/2017/03/03/map-merge/index.html
_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries



--
-Andrew Thaddeus Martin

_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

Chris Wong-2
In reply to this post by David Feuer
Hi David,

On Feb 14, 2018 08:34, "David Feuer" <[hidden email]> wrote:
Alternatives:

1. Remove the instances but don't add the new ones. I fear this may
lead others to write their own orphan instances, which may not even
all do the same thing.

2. Write separate modules with newtype-wrapped versions of the data
structures implementing the desired instances. Unfortunately, this
would be annoying to maintain, and also annoying to use--packages
using the unwrapped and wrapped versions will use different types.
Manually wrapping and unwrapping to make the different types work with
each other will introduce lots of potential for mistakes and
confusion.

A third alternative is to "poison" the instances by placing TypeError in their constraints. This would prevent users from writing orphan instances, as they would overlap with the poisoned ones.

On other Haskell implementations (if we still support them), we can use a private class with no instances for this purpose.


Chris


Discussion period: three weeks.

[*] http://teh.id.au/posts/2017/03/03/map-merge/index.html
_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries


_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

Mario Blažević
In reply to this post by David Feuer
+1. But whatever happened to your proposal from last May? I don't think
there were any objections to it. Would the two proposals be combined, or
have you decided to drop the previous one?

https://mail.haskell.org/pipermail/libraries/2017-May/028036.html

On 2017-05-25 12:55 PM, David Feuer wrote:
 > A lot of people have wrappers around Data.Map and Data.IntMap to give
 > them more useful (Semigroup and) Monoid instances. I'd like to add such
 > wrappers to containers. What we need to be able to do that are *names*
 > for the new modules. I can't think of any, so I'm reaching out to the
 > list. Please suggest names! Another question is whether we should take
 > the opportunity of new modules to modernize and streamline the API a
 > bit. I'd like, at least, to separate "safe" from "unsafe" functions,
 > putting the unsafe ones in .Unsafe modules.


On 2018-02-13 02:33 PM, David Feuer wrote:

> Many people have recognized for years that the Semigroup and Monoid
> instances for Data.Map, Data.IntMap, and Data.HashMap are not so
> great. In particular, when the same key is present in both maps, they
> simply use the value from the first argument, ignoring the other one.
> This somewhat counter-intuitive behavior can lead to bugs. See, for
> example, the discussion in Tim Humphries's blog post[*]. I would like
> do do the following:
>
> 1. Deprecate the Semigroup and Monoid instances for Data.Map.Map,
> Data.IntMap.IntMap, and Data.HashMap.HashMap in the next releases of
> containers and unordered-containers.
>
> 2. Remove the deprecated instances.
>
> 3. After another several years (four or five, perhaps?), make a major
> release of each package in which the instances are replaced with the
> following:
>
>    instance (Ord k, Semigroup v) => Semigroup (Map k v) where
>      (<>) = Data.Map.Strict.unionWith (<>)
>    instance (Ord k, Semigroup v) => Monoid (Map k v) where
>      mempty = Data.Map.Strict.empty
>
>    instance Semigroup v => Semigroup (IntMap v) where
>      (<>) = Data.IntMap.Strict.unionWith (<>)
>    instance Semigroup v => Monoid (IntMap v) where
>      mempty = Data.IntMap.Strict.empty
>
>    instance (Eq k, Hashable k, Semigroup v) => Semigroup (HashMap k v) where
>      (<>) = Data.HashMap.Strict.unionWith (<>)
>    instance (Eq k, Hashable k, Semigroup v) => Monoid(HashMap k v) where
>      mempty = Data.HashMap.Strict.empty
>
> Why do I want the strict versions? That choice may seem a bit
> surprising, since the data structures are lazy. But the lazy versions
> really like to leak memory, making them unsuitable for most practical
> purposes.
>
> The big risk:
>
> Someone using old code or old tutorial documentation could get subtly
> wrong behavior without noticing. That is why I have specified an
> extended period between removing the current instances and adding the
> desired ones.
>
> Alternatives:
>
> 1. Remove the instances but don't add the new ones. I fear this may
> lead others to write their own orphan instances, which may not even
> all do the same thing.
>
> 2. Write separate modules with newtype-wrapped versions of the data
> structures implementing the desired instances. Unfortunately, this
> would be annoying to maintain, and also annoying to use--packages
> using the unwrapped and wrapped versions will use different types.
> Manually wrapping and unwrapping to make the different types work with
> each other will introduce lots of potential for mistakes and
> confusion.
>
> Discussion period: three weeks.
>
> [*] http://teh.id.au/posts/2017/03/03/map-merge/index.html
> _______________________________________________
> Libraries mailing list
> [hidden email]
> http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
>

_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

Kris Nuttycombe
In reply to this post by David Feuer
I actually had this very issue bite me *in an interview*. I would love these instances to be removed, and probably not replaced directly - instead, I think we should take the `Sum`/`Product` approach, and provide instances for newtypes around these type constructors so that there can be no backwards-incompatible-semantics issues.

On Tue, Feb 13, 2018 at 12:33 PM, David Feuer <[hidden email]> wrote:
Many people have recognized for years that the Semigroup and Monoid
instances for Data.Map, Data.IntMap, and Data.HashMap are not so
great. In particular, when the same key is present in both maps, they
simply use the value from the first argument, ignoring the other one.
This somewhat counter-intuitive behavior can lead to bugs. See, for
example, the discussion in Tim Humphries's blog post[*]. I would like
do do the following:

1. Deprecate the Semigroup and Monoid instances for Data.Map.Map,
Data.IntMap.IntMap, and Data.HashMap.HashMap in the next releases of
containers and unordered-containers.

2. Remove the deprecated instances.

3. After another several years (four or five, perhaps?), make a major
release of each package in which the instances are replaced with the
following:

  instance (Ord k, Semigroup v) => Semigroup (Map k v) where
    (<>) = Data.Map.Strict.unionWith (<>)
  instance (Ord k, Semigroup v) => Monoid (Map k v) where
    mempty = Data.Map.Strict.empty

  instance Semigroup v => Semigroup (IntMap v) where
    (<>) = Data.IntMap.Strict.unionWith (<>)
  instance Semigroup v => Monoid (IntMap v) where
    mempty = Data.IntMap.Strict.empty

  instance (Eq k, Hashable k, Semigroup v) => Semigroup (HashMap k v) where
    (<>) = Data.HashMap.Strict.unionWith (<>)
  instance (Eq k, Hashable k, Semigroup v) => Monoid(HashMap k v) where
    mempty = Data.HashMap.Strict.empty

Why do I want the strict versions? That choice may seem a bit
surprising, since the data structures are lazy. But the lazy versions
really like to leak memory, making them unsuitable for most practical
purposes.

The big risk:

Someone using old code or old tutorial documentation could get subtly
wrong behavior without noticing. That is why I have specified an
extended period between removing the current instances and adding the
desired ones.

Alternatives:

1. Remove the instances but don't add the new ones. I fear this may
lead others to write their own orphan instances, which may not even
all do the same thing.

2. Write separate modules with newtype-wrapped versions of the data
structures implementing the desired instances. Unfortunately, this
would be annoying to maintain, and also annoying to use--packages
using the unwrapped and wrapped versions will use different types.
Manually wrapping and unwrapping to make the different types work with
each other will introduce lots of potential for mistakes and
confusion.

Discussion period: three weeks.

[*] http://teh.id.au/posts/2017/03/03/map-merge/index.html
_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries


_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

David Feuer
In reply to this post by Mario Blažević
I don't think the old proposal was really the right way.

On Tue, Feb 13, 2018 at 2:54 PM, Mario Blažević <[hidden email]> wrote:

> +1. But whatever happened to your proposal from last May? I don't think
> there were any objections to it. Would the two proposals be combined, or
> have you decided to drop the previous one?
>
> https://mail.haskell.org/pipermail/libraries/2017-May/028036.html
>
> On 2017-05-25 12:55 PM, David Feuer wrote:
>> A lot of people have wrappers around Data.Map and Data.IntMap to give
>> them more useful (Semigroup and) Monoid instances. I'd like to add such
>> wrappers to containers. What we need to be able to do that are *names*
>> for the new modules. I can't think of any, so I'm reaching out to the
>> list. Please suggest names! Another question is whether we should take
>> the opportunity of new modules to modernize and streamline the API a
>> bit. I'd like, at least, to separate "safe" from "unsafe" functions,
>> putting the unsafe ones in .Unsafe modules.
>
>
>
> On 2018-02-13 02:33 PM, David Feuer wrote:
>>
>> Many people have recognized for years that the Semigroup and Monoid
>> instances for Data.Map, Data.IntMap, and Data.HashMap are not so
>> great. In particular, when the same key is present in both maps, they
>> simply use the value from the first argument, ignoring the other one.
>> This somewhat counter-intuitive behavior can lead to bugs. See, for
>> example, the discussion in Tim Humphries's blog post[*]. I would like
>> do do the following:
>>
>> 1. Deprecate the Semigroup and Monoid instances for Data.Map.Map,
>> Data.IntMap.IntMap, and Data.HashMap.HashMap in the next releases of
>> containers and unordered-containers.
>>
>> 2. Remove the deprecated instances.
>>
>> 3. After another several years (four or five, perhaps?), make a major
>> release of each package in which the instances are replaced with the
>> following:
>>
>>    instance (Ord k, Semigroup v) => Semigroup (Map k v) where
>>      (<>) = Data.Map.Strict.unionWith (<>)
>>    instance (Ord k, Semigroup v) => Monoid (Map k v) where
>>      mempty = Data.Map.Strict.empty
>>
>>    instance Semigroup v => Semigroup (IntMap v) where
>>      (<>) = Data.IntMap.Strict.unionWith (<>)
>>    instance Semigroup v => Monoid (IntMap v) where
>>      mempty = Data.IntMap.Strict.empty
>>
>>    instance (Eq k, Hashable k, Semigroup v) => Semigroup (HashMap k v)
>> where
>>      (<>) = Data.HashMap.Strict.unionWith (<>)
>>    instance (Eq k, Hashable k, Semigroup v) => Monoid(HashMap k v) where
>>      mempty = Data.HashMap.Strict.empty
>>
>> Why do I want the strict versions? That choice may seem a bit
>> surprising, since the data structures are lazy. But the lazy versions
>> really like to leak memory, making them unsuitable for most practical
>> purposes.
>>
>> The big risk:
>>
>> Someone using old code or old tutorial documentation could get subtly
>> wrong behavior without noticing. That is why I have specified an
>> extended period between removing the current instances and adding the
>> desired ones.
>>
>> Alternatives:
>>
>> 1. Remove the instances but don't add the new ones. I fear this may
>> lead others to write their own orphan instances, which may not even
>> all do the same thing.
>>
>> 2. Write separate modules with newtype-wrapped versions of the data
>> structures implementing the desired instances. Unfortunately, this
>> would be annoying to maintain, and also annoying to use--packages
>> using the unwrapped and wrapped versions will use different types.
>> Manually wrapping and unwrapping to make the different types work with
>> each other will introduce lots of potential for mistakes and
>> confusion.
>>
>> Discussion period: three weeks.
>>
>> [*] http://teh.id.au/posts/2017/03/03/map-merge/index.html
>> _______________________________________________
>> Libraries mailing list
>> [hidden email]
>> http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
>>
>
> _______________________________________________
> Libraries mailing list
> [hidden email]
> http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

Daniel Cartwright
I am strongly in favour of this as well. I do not like the newtype approach, I think backwards-*incompatibility* with four to five year-old (or two to three year-old) code is OK.

On Tue, Feb 13, 2018 at 3:04 PM, Daniel Cartwright <[hidden email]> wrote:
I am strongly in favour of this as well. I do not like the newtype approach, I think backwards-*incompatibility* with four to five year-old (or two to three year-old) code is OK.

On Tue, Feb 13, 2018 at 2:59 PM, David Feuer <[hidden email]> wrote:
I don't think the old proposal was really the right way.

On Tue, Feb 13, 2018 at 2:54 PM, Mario Blažević <[hidden email]> wrote:
> +1. But whatever happened to your proposal from last May? I don't think
> there were any objections to it. Would the two proposals be combined, or
> have you decided to drop the previous one?
>
> https://mail.haskell.org/pipermail/libraries/2017-May/028036.html
>
> On 2017-05-25 12:55 PM, David Feuer wrote:
>> A lot of people have wrappers around Data.Map and Data.IntMap to give
>> them more useful (Semigroup and) Monoid instances. I'd like to add such
>> wrappers to containers. What we need to be able to do that are *names*
>> for the new modules. I can't think of any, so I'm reaching out to the
>> list. Please suggest names! Another question is whether we should take
>> the opportunity of new modules to modernize and streamline the API a
>> bit. I'd like, at least, to separate "safe" from "unsafe" functions,
>> putting the unsafe ones in .Unsafe modules.
>
>
>
> On 2018-02-13 02:33 PM, David Feuer wrote:
>>
>> Many people have recognized for years that the Semigroup and Monoid
>> instances for Data.Map, Data.IntMap, and Data.HashMap are not so
>> great. In particular, when the same key is present in both maps, they
>> simply use the value from the first argument, ignoring the other one.
>> This somewhat counter-intuitive behavior can lead to bugs. See, for
>> example, the discussion in Tim Humphries's blog post[*]. I would like
>> do do the following:
>>
>> 1. Deprecate the Semigroup and Monoid instances for Data.Map.Map,
>> Data.IntMap.IntMap, and Data.HashMap.HashMap in the next releases of
>> containers and unordered-containers.
>>
>> 2. Remove the deprecated instances.
>>
>> 3. After another several years (four or five, perhaps?), make a major
>> release of each package in which the instances are replaced with the
>> following:
>>
>>    instance (Ord k, Semigroup v) => Semigroup (Map k v) where
>>      (<>) = Data.Map.Strict.unionWith (<>)
>>    instance (Ord k, Semigroup v) => Monoid (Map k v) where
>>      mempty = Data.Map.Strict.empty
>>
>>    instance Semigroup v => Semigroup (IntMap v) where
>>      (<>) = Data.IntMap.Strict.unionWith (<>)
>>    instance Semigroup v => Monoid (IntMap v) where
>>      mempty = Data.IntMap.Strict.empty
>>
>>    instance (Eq k, Hashable k, Semigroup v) => Semigroup (HashMap k v)
>> where
>>      (<>) = Data.HashMap.Strict.unionWith (<>)
>>    instance (Eq k, Hashable k, Semigroup v) => Monoid(HashMap k v) where
>>      mempty = Data.HashMap.Strict.empty
>>
>> Why do I want the strict versions? That choice may seem a bit
>> surprising, since the data structures are lazy. But the lazy versions
>> really like to leak memory, making them unsuitable for most practical
>> purposes.
>>
>> The big risk:
>>
>> Someone using old code or old tutorial documentation could get subtly
>> wrong behavior without noticing. That is why I have specified an
>> extended period between removing the current instances and adding the
>> desired ones.
>>
>> Alternatives:
>>
>> 1. Remove the instances but don't add the new ones. I fear this may
>> lead others to write their own orphan instances, which may not even
>> all do the same thing.
>>
>> 2. Write separate modules with newtype-wrapped versions of the data
>> structures implementing the desired instances. Unfortunately, this
>> would be annoying to maintain, and also annoying to use--packages
>> using the unwrapped and wrapped versions will use different types.
>> Manually wrapping and unwrapping to make the different types work with
>> each other will introduce lots of potential for mistakes and
>> confusion.
>>
>> Discussion period: three weeks.
>>
>> [*] http://teh.id.au/posts/2017/03/03/map-merge/index.html
>> _______________________________________________
>> Libraries mailing list
>> [hidden email]
>> http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
>>
>
> _______________________________________________
> Libraries mailing list
> [hidden email]
> http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries



_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

Kris Nuttycombe
Given that we're talking about a years-long process, what about using the newtype approach as an intermediate step, so that we don't get additional proliferation of third-party newtype wrappers in the interim? Deprecate the current instances and add instances using newtype wrappers at the same type, such that a newtype'd version of the current functionality is available to folks as a migration path, as well as a newtype for which the instances provide the correct behavior. Then, in the future, the newtypes for the corrected behavior will eventually become superfluous, and for those who rely on the newtypes that represent the current behavior, they won't have to change anything.

On Tue, Feb 13, 2018 at 1:07 PM, Daniel Cartwright <[hidden email]> wrote:
I am strongly in favour of this as well. I do not like the newtype approach, I think backwards-*incompatibility* with four to five year-old (or two to three year-old) code is OK.

On Tue, Feb 13, 2018 at 3:04 PM, Daniel Cartwright <[hidden email]> wrote:
I am strongly in favour of this as well. I do not like the newtype approach, I think backwards-*incompatibility* with four to five year-old (or two to three year-old) code is OK.

On Tue, Feb 13, 2018 at 2:59 PM, David Feuer <[hidden email]> wrote:
I don't think the old proposal was really the right way.

On Tue, Feb 13, 2018 at 2:54 PM, Mario Blažević <[hidden email]> wrote:
> +1. But whatever happened to your proposal from last May? I don't think
> there were any objections to it. Would the two proposals be combined, or
> have you decided to drop the previous one?
>
> https://mail.haskell.org/pipermail/libraries/2017-May/028036.html
>
> On 2017-05-25 12:55 PM, David Feuer wrote:
>> A lot of people have wrappers around Data.Map and Data.IntMap to give
>> them more useful (Semigroup and) Monoid instances. I'd like to add such
>> wrappers to containers. What we need to be able to do that are *names*
>> for the new modules. I can't think of any, so I'm reaching out to the
>> list. Please suggest names! Another question is whether we should take
>> the opportunity of new modules to modernize and streamline the API a
>> bit. I'd like, at least, to separate "safe" from "unsafe" functions,
>> putting the unsafe ones in .Unsafe modules.
>
>
>
> On 2018-02-13 02:33 PM, David Feuer wrote:
>>
>> Many people have recognized for years that the Semigroup and Monoid
>> instances for Data.Map, Data.IntMap, and Data.HashMap are not so
>> great. In particular, when the same key is present in both maps, they
>> simply use the value from the first argument, ignoring the other one.
>> This somewhat counter-intuitive behavior can lead to bugs. See, for
>> example, the discussion in Tim Humphries's blog post[*]. I would like
>> do do the following:
>>
>> 1. Deprecate the Semigroup and Monoid instances for Data.Map.Map,
>> Data.IntMap.IntMap, and Data.HashMap.HashMap in the next releases of
>> containers and unordered-containers.
>>
>> 2. Remove the deprecated instances.
>>
>> 3. After another several years (four or five, perhaps?), make a major
>> release of each package in which the instances are replaced with the
>> following:
>>
>>    instance (Ord k, Semigroup v) => Semigroup (Map k v) where
>>      (<>) = Data.Map.Strict.unionWith (<>)
>>    instance (Ord k, Semigroup v) => Monoid (Map k v) where
>>      mempty = Data.Map.Strict.empty
>>
>>    instance Semigroup v => Semigroup (IntMap v) where
>>      (<>) = Data.IntMap.Strict.unionWith (<>)
>>    instance Semigroup v => Monoid (IntMap v) where
>>      mempty = Data.IntMap.Strict.empty
>>
>>    instance (Eq k, Hashable k, Semigroup v) => Semigroup (HashMap k v)
>> where
>>      (<>) = Data.HashMap.Strict.unionWith (<>)
>>    instance (Eq k, Hashable k, Semigroup v) => Monoid(HashMap k v) where
>>      mempty = Data.HashMap.Strict.empty
>>
>> Why do I want the strict versions? That choice may seem a bit
>> surprising, since the data structures are lazy. But the lazy versions
>> really like to leak memory, making them unsuitable for most practical
>> purposes.
>>
>> The big risk:
>>
>> Someone using old code or old tutorial documentation could get subtly
>> wrong behavior without noticing. That is why I have specified an
>> extended period between removing the current instances and adding the
>> desired ones.
>>
>> Alternatives:
>>
>> 1. Remove the instances but don't add the new ones. I fear this may
>> lead others to write their own orphan instances, which may not even
>> all do the same thing.
>>
>> 2. Write separate modules with newtype-wrapped versions of the data
>> structures implementing the desired instances. Unfortunately, this
>> would be annoying to maintain, and also annoying to use--packages
>> using the unwrapped and wrapped versions will use different types.
>> Manually wrapping and unwrapping to make the different types work with
>> each other will introduce lots of potential for mistakes and
>> confusion.
>>
>> Discussion period: three weeks.
>>
>> [*] http://teh.id.au/posts/2017/03/03/map-merge/index.html
>> _______________________________________________
>> Libraries mailing list
>> [hidden email]
>> http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
>>
>
> _______________________________________________
> Libraries mailing list
> [hidden email]
> http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries



_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries



_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

John Wiegley-2
In reply to this post by Andrew Martin
>>>>> "AM" == Andrew Martin <[hidden email]> writes:

AM> I am strongly in favor of this proposal. I've actually proposal the same
AM> thing (with additional motivations) on the GHC Trac: https://
AM> ghc.haskell.org/trac/ghc/ticket/1460

+1 here too

--
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2
_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

David Feuer
In reply to this post by Kris Nuttycombe
I don't want to maintain a whole package of newtype-wrapped everything
for this, but I gather that Ben Gamari already has one people can use
if they like. Now that Semigroup is in base, I think Kris's idea of
using newtype wrappers *permanently* might be a good idea. That way we
can have

newtype UnionCombine k v = UnionCombine (Map k v)
UnionCombine xs <> UnionCombine ys = UnionCombine $ unionWith (<>) xs ys

newtype IntersectionCombine k v = IntersectionCombine (Map k v)
IntersectionCombine xs <> IntersectionCombine ys = IntersectionCombine
$ intersectionWith (<>) xs ys

We can add such newtypes (by whatever names people prefer)
immediately, and kick the question of whether to ever reinstate the
instances down the road.

On Tue, Feb 13, 2018 at 3:16 PM, Kris Nuttycombe
<[hidden email]> wrote:

> Given that we're talking about a years-long process, what about using the
> newtype approach as an intermediate step, so that we don't get additional
> proliferation of third-party newtype wrappers in the interim? Deprecate the
> current instances and add instances using newtype wrappers at the same type,
> such that a newtype'd version of the current functionality is available to
> folks as a migration path, as well as a newtype for which the instances
> provide the correct behavior. Then, in the future, the newtypes for the
> corrected behavior will eventually become superfluous, and for those who
> rely on the newtypes that represent the current behavior, they won't have to
> change anything.
>
> On Tue, Feb 13, 2018 at 1:07 PM, Daniel Cartwright <[hidden email]>
> wrote:
>>
>> I am strongly in favour of this as well. I do not like the newtype
>> approach, I think backwards-*incompatibility* with four to five year-old (or
>> two to three year-old) code is OK.
>>
>> On Tue, Feb 13, 2018 at 3:04 PM, Daniel Cartwright <[hidden email]>
>> wrote:
>>>
>>> I am strongly in favour of this as well. I do not like the newtype
>>> approach, I think backwards-*incompatibility* with four to five year-old (or
>>> two to three year-old) code is OK.
>>>
>>> On Tue, Feb 13, 2018 at 2:59 PM, David Feuer <[hidden email]>
>>> wrote:
>>>>
>>>> I don't think the old proposal was really the right way.
>>>>
>>>> On Tue, Feb 13, 2018 at 2:54 PM, Mario Blažević <[hidden email]>
>>>> wrote:
>>>> > +1. But whatever happened to your proposal from last May? I don't
>>>> > think
>>>> > there were any objections to it. Would the two proposals be combined,
>>>> > or
>>>> > have you decided to drop the previous one?
>>>> >
>>>> > https://mail.haskell.org/pipermail/libraries/2017-May/028036.html
>>>> >
>>>> > On 2017-05-25 12:55 PM, David Feuer wrote:
>>>> >> A lot of people have wrappers around Data.Map and Data.IntMap to give
>>>> >> them more useful (Semigroup and) Monoid instances. I'd like to add
>>>> >> such
>>>> >> wrappers to containers. What we need to be able to do that are
>>>> >> *names*
>>>> >> for the new modules. I can't think of any, so I'm reaching out to the
>>>> >> list. Please suggest names! Another question is whether we should
>>>> >> take
>>>> >> the opportunity of new modules to modernize and streamline the API a
>>>> >> bit. I'd like, at least, to separate "safe" from "unsafe" functions,
>>>> >> putting the unsafe ones in .Unsafe modules.
>>>> >
>>>> >
>>>> >
>>>> > On 2018-02-13 02:33 PM, David Feuer wrote:
>>>> >>
>>>> >> Many people have recognized for years that the Semigroup and Monoid
>>>> >> instances for Data.Map, Data.IntMap, and Data.HashMap are not so
>>>> >> great. In particular, when the same key is present in both maps, they
>>>> >> simply use the value from the first argument, ignoring the other one.
>>>> >> This somewhat counter-intuitive behavior can lead to bugs. See, for
>>>> >> example, the discussion in Tim Humphries's blog post[*]. I would like
>>>> >> do do the following:
>>>> >>
>>>> >> 1. Deprecate the Semigroup and Monoid instances for Data.Map.Map,
>>>> >> Data.IntMap.IntMap, and Data.HashMap.HashMap in the next releases of
>>>> >> containers and unordered-containers.
>>>> >>
>>>> >> 2. Remove the deprecated instances.
>>>> >>
>>>> >> 3. After another several years (four or five, perhaps?), make a major
>>>> >> release of each package in which the instances are replaced with the
>>>> >> following:
>>>> >>
>>>> >>    instance (Ord k, Semigroup v) => Semigroup (Map k v) where
>>>> >>      (<>) = Data.Map.Strict.unionWith (<>)
>>>> >>    instance (Ord k, Semigroup v) => Monoid (Map k v) where
>>>> >>      mempty = Data.Map.Strict.empty
>>>> >>
>>>> >>    instance Semigroup v => Semigroup (IntMap v) where
>>>> >>      (<>) = Data.IntMap.Strict.unionWith (<>)
>>>> >>    instance Semigroup v => Monoid (IntMap v) where
>>>> >>      mempty = Data.IntMap.Strict.empty
>>>> >>
>>>> >>    instance (Eq k, Hashable k, Semigroup v) => Semigroup (HashMap k
>>>> >> v)
>>>> >> where
>>>> >>      (<>) = Data.HashMap.Strict.unionWith (<>)
>>>> >>    instance (Eq k, Hashable k, Semigroup v) => Monoid(HashMap k v)
>>>> >> where
>>>> >>      mempty = Data.HashMap.Strict.empty
>>>> >>
>>>> >> Why do I want the strict versions? That choice may seem a bit
>>>> >> surprising, since the data structures are lazy. But the lazy versions
>>>> >> really like to leak memory, making them unsuitable for most practical
>>>> >> purposes.
>>>> >>
>>>> >> The big risk:
>>>> >>
>>>> >> Someone using old code or old tutorial documentation could get subtly
>>>> >> wrong behavior without noticing. That is why I have specified an
>>>> >> extended period between removing the current instances and adding the
>>>> >> desired ones.
>>>> >>
>>>> >> Alternatives:
>>>> >>
>>>> >> 1. Remove the instances but don't add the new ones. I fear this may
>>>> >> lead others to write their own orphan instances, which may not even
>>>> >> all do the same thing.
>>>> >>
>>>> >> 2. Write separate modules with newtype-wrapped versions of the data
>>>> >> structures implementing the desired instances. Unfortunately, this
>>>> >> would be annoying to maintain, and also annoying to use--packages
>>>> >> using the unwrapped and wrapped versions will use different types.
>>>> >> Manually wrapping and unwrapping to make the different types work
>>>> >> with
>>>> >> each other will introduce lots of potential for mistakes and
>>>> >> confusion.
>>>> >>
>>>> >> Discussion period: three weeks.
>>>> >>
>>>> >> [*] http://teh.id.au/posts/2017/03/03/map-merge/index.html
>>>> >> _______________________________________________
>>>> >> Libraries mailing list
>>>> >> [hidden email]
>>>> >> http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
>>>> >>
>>>> >
>>>> > _______________________________________________
>>>> > Libraries mailing list
>>>> > [hidden email]
>>>> > http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
>>>> _______________________________________________
>>>> Libraries mailing list
>>>> [hidden email]
>>>> http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
>>>
>>>
>>
>>
>> _______________________________________________
>> Libraries mailing list
>> [hidden email]
>> http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
>>
>
_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

Herbert Valerio Riedel-3
In reply to this post by David Feuer
David,


On 2018-02-13 at 14:33:45 -0500, David Feuer wrote:

[...]

> 1. Deprecate the Semigroup and Monoid instances for Data.Map.Map,
> Data.IntMap.IntMap, and Data.HashMap.HashMap in the next releases of
> containers and unordered-containers.
>
> 2. Remove the deprecated instances.
>
> 3. After another several years (four or five, perhaps?), make a major
> release of each package in which the instances are replaced with the
> following:

Why does this need to be a such a dreadfully long-winded process? All we
need is a way to somehow signal a semantic change in the exposed API --
and it turns out that we actually already have the technology for this!

This is exactly one of the core ideas of "semantic versioning" (as a
dep-solver-computation-friendly proxy for machine-checkable formal API
contracts...) and it's got even "semantic" in its name!


In other words, the proposal can be greatly simplified to


1. Make a new major release (or maybe even make it a super-major
   release, i.e. to `containers-1.0.0.0` for extra saliency) replacing
   the instances with the more desirable ones; describe the change
   prominently in the changelog.


Profit!


Life's short; do we really need a multi-generational journey where the
original supporters may not even be around anymore to see the proposal
reach its final destination... ;-)


Cheers,
  HVR
_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

m-renaud
+1 to HVR's suggestion, this could be a great opportunity to clean out all the old cruft from the containers and unordered-containers APIs in one go. That combined with all the documentation improvements would warrant a 1.0.0.0 for both packages IMHO :) I'd be happy to help with figuring out what that 1.0.0.0 API could/should look like, since it should almost certainly include a lot of community input/feedback.

On Tue, Feb 13, 2018 at 2:28 PM, Herbert Valerio Riedel <[hidden email]> wrote:
David,


On 2018-02-13 at 14:33:45 -0500, David Feuer wrote:

[...]

> 1. Deprecate the Semigroup and Monoid instances for Data.Map.Map,
> Data.IntMap.IntMap, and Data.HashMap.HashMap in the next releases of
> containers and unordered-containers.
>
> 2. Remove the deprecated instances.
>
> 3. After another several years (four or five, perhaps?), make a major
> release of each package in which the instances are replaced with the
> following:

Why does this need to be a such a dreadfully long-winded process? All we
need is a way to somehow signal a semantic change in the exposed API --
and it turns out that we actually already have the technology for this!

This is exactly one of the core ideas of "semantic versioning" (as a
dep-solver-computation-friendly proxy for machine-checkable formal API
contracts...) and it's got even "semantic" in its name!


In other words, the proposal can be greatly simplified to


1. Make a new major release (or maybe even make it a super-major
   release, i.e. to `containers-1.0.0.0` for extra saliency) replacing
   the instances with the more desirable ones; describe the change
   prominently in the changelog.


Profit!


Life's short; do we really need a multi-generational journey where the
original supporters may not even be around anymore to see the proposal
reach its final destination... ;-)


Cheers,
  HVR
_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries


_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

David Feuer
In reply to this post by Herbert Valerio Riedel-3
We may not have to be *quite* as long-winded as I initially suggested,
but I don't think your way is sufficiently long-winded. The advantage
of removing the instances first and then adding them back is that the
version(s) without the instances will make compilation of every single
package that uses them fail. The way you suggest, if someone has a
package using containers or unordered-containers, they don't even have
a simple way to find out whether they need to make changes. On the
opposite side, Chris Wong's suggestion would let us be very explicit,
with a type error that warns users that the instance is missing
*because it is changing* and that no code using the instance will work
for both 0.* and 1.* versions. But I do think we should also consider
Kris's more conservative approach.

On Tue, Feb 13, 2018 at 5:28 PM, Herbert Valerio Riedel
<[hidden email]> wrote:

> David,
>
>
> On 2018-02-13 at 14:33:45 -0500, David Feuer wrote:
>
> [...]
>
>> 1. Deprecate the Semigroup and Monoid instances for Data.Map.Map,
>> Data.IntMap.IntMap, and Data.HashMap.HashMap in the next releases of
>> containers and unordered-containers.
>>
>> 2. Remove the deprecated instances.
>>
>> 3. After another several years (four or five, perhaps?), make a major
>> release of each package in which the instances are replaced with the
>> following:
>
> Why does this need to be a such a dreadfully long-winded process? All we
> need is a way to somehow signal a semantic change in the exposed API --
> and it turns out that we actually already have the technology for this!
>
> This is exactly one of the core ideas of "semantic versioning" (as a
> dep-solver-computation-friendly proxy for machine-checkable formal API
> contracts...) and it's got even "semantic" in its name!
>
>
> In other words, the proposal can be greatly simplified to
>
>
> 1. Make a new major release (or maybe even make it a super-major
>    release, i.e. to `containers-1.0.0.0` for extra saliency) replacing
>    the instances with the more desirable ones; describe the change
>    prominently in the changelog.
>
>
> Profit!
>
>
> Life's short; do we really need a multi-generational journey where the
> original supporters may not even be around anymore to see the proposal
> reach its final destination... ;-)
>
>
> Cheers,
>   HVR
_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

Herbert Valerio Riedel-3
Hi,

On 2018-02-13 at 17:41:25 -0500, David Feuer wrote:

> We may not have to be *quite* as long-winded as I initially suggested,
> but I don't think your way is sufficiently long-winded. The advantage
> of removing the instances first and then adding them back is that the
> version(s) without the instances will make compilation of every single
> package that uses them fail. The way you suggest, if someone has a
> package using containers or unordered-containers, they don't even have
> a simple way to find out whether they need to make changes. On the
> opposite side, Chris Wong's suggestion would let us be very explicit,
> with a type error that warns users that the instance is missing
> *because it is changing* and that no code using the instance will work
> for both 0.* and 1.* versions.

Alright, then let's do a little Gedankenexperiment; what if you release
a containers-0.9.0.0 (which drops the instances) and a
containers-1.0.0.0 (which 'adds back' the desired new instances) on the
same day!

...wouldn't this allow us to have the cake and eat it too? ;-)
_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

David Feuer
It would let us have some cake. Users would be able to test against 0.9, in theory. But they'd have to do it intentionally. And Stack-based projects would probably need some shenanigans to deal with a version of containers that doesn't come with GHC. So I really think that avoiding these subtle bugs requires at least a full GHC release cycle.

On Feb 13, 2018 5:48 PM, "Herbert Valerio Riedel" <[hidden email]> wrote:
Hi,

On 2018-02-13 at 17:41:25 -0500, David Feuer wrote:
> We may not have to be *quite* as long-winded as I initially suggested,
> but I don't think your way is sufficiently long-winded. The advantage
> of removing the instances first and then adding them back is that the
> version(s) without the instances will make compilation of every single
> package that uses them fail. The way you suggest, if someone has a
> package using containers or unordered-containers, they don't even have
> a simple way to find out whether they need to make changes. On the
> opposite side, Chris Wong's suggestion would let us be very explicit,
> with a type error that warns users that the instance is missing
> *because it is changing* and that no code using the instance will work
> for both 0.* and 1.* versions.

Alright, then let's do a little Gedankenexperiment; what if you release
a containers-0.9.0.0 (which drops the instances) and a
containers-1.0.0.0 (which 'adds back' the desired new instances) on the
same day!

...wouldn't this allow us to have the cake and eat it too? ;-)


_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

m-renaud
> avoiding these subtle bugs requires at least a full GHC release cycle

Now that GHC is on a 6 month release cycle this seems preferable to waiting 3-5 years.


On Tue, Feb 13, 2018 at 2:52 PM, David Feuer <[hidden email]> wrote:
It would let us have some cake. Users would be able to test against 0.9, in theory. But they'd have to do it intentionally. And Stack-based projects would probably need some shenanigans to deal with a version of containers that doesn't come with GHC. So I really think that avoiding these subtle bugs requires at least a full GHC release cycle.

On Feb 13, 2018 5:48 PM, "Herbert Valerio Riedel" <[hidden email]> wrote:
Hi,

On 2018-02-13 at 17:41:25 -0500, David Feuer wrote:
> We may not have to be *quite* as long-winded as I initially suggested,
> but I don't think your way is sufficiently long-winded. The advantage
> of removing the instances first and then adding them back is that the
> version(s) without the instances will make compilation of every single
> package that uses them fail. The way you suggest, if someone has a
> package using containers or unordered-containers, they don't even have
> a simple way to find out whether they need to make changes. On the
> opposite side, Chris Wong's suggestion would let us be very explicit,
> with a type error that warns users that the instance is missing
> *because it is changing* and that no code using the instance will work
> for both 0.* and 1.* versions.

Alright, then let's do a little Gedankenexperiment; what if you release
a containers-0.9.0.0 (which drops the instances) and a
containers-1.0.0.0 (which 'adds back' the desired new instances) on the
same day!

...wouldn't this allow us to have the cake and eat it too? ;-)


_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries



_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

M Farkas-Dyck-2
In reply to this post by David Feuer
I am strongly in favor of parts 1 and 2, and HVR's proposal for
immediately defining new instances with a major version bump — this is
what major versions are for, and if we leave them undefined, it will
encourage users to define orphan instances.
_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

Herbert Valerio Riedel-3
In reply to this post by David Feuer
David,

>> Alright, then let's do a little Gedankenexperiment; what if you release
>> a containers-0.9.0.0 (which drops the instances) and a
>> containers-1.0.0.0 (which 'adds back' the desired new instances) on the
>> same day!
>>
>> ...wouldn't this allow us to have the cake and eat it too? ;-)

> It would let us have some cake. Users would be able to test against
> 0.9, in theory. But they'd have to do it intentionally.

This is what I was trying to get at: that's always the case. It doesn't
matter whether you release it same day, a week later, or a year later. I
have to intentionally *not* skip the 0.9 release. In fact, I'd want to
skip the 0.9 release myself if possible, as supporting both,
containers-0.9 and containers-1.0 in the same consumer code is going to
be awkward enough for me not wanting to do that (or I'd have to
basically not use the instances at all); If I care about the new
instances, I'd rather specify `containers ^>= 1.0.0.0` and thus not
support containers-0.9.

I would have proceeded to point out in my Gedankenexperiment, that in
order to use such a container-0.9 release, you'd have to force every
package that depends on `containers` to go through such a 0.9 phase in
lockstep, which in itself poses an ordeal in terms of package update
logistic; and then if you spread the time between the 0.9 and the 1.0
release apart, repeat this dance a 2nd time, with yet another new major
version bump, which requires yet another round of consumer-code reviews
(*because* a major version increment was signalled; and we do that as
API consumers are supposed to pay attention to the major version
increment signal...)

So consequently, if anything, a container-0.9 release would be a kind of
an artificial pseudo-release anyway IMO. You could even just condense it
into a cabal package flag of a containers-1.0 release, which disables
the Monoid/Semigroup instances; then you don't even need a distinct
container-0.9 release at all, nor do you have to contaminate the API
progression with an artificial container-0.9 discontinuity.

> And Stack-based projects would probably need some shenanigans to deal
> with a version of containers that doesn't come with GHC.

I'm not convinced we should let the weakest link hold us back. If
there's limitations in the tooling, we should rather address them, rather
than contort ourselves; c.f. the Genius Tailor
( http://www.wealthculture.cn/infoShow.asp?nid=880&sort=Article%20List )

> So I really think that avoiding these subtle bugs requires at least a
> full GHC release cycle.

I don't think waiting a full GHC release would be either necessary nor
effective at addressing your concerns, which I consider to be disputable
arguments to begin with.


-- hvr
_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

Henning Thielemann
In reply to this post by David Feuer

On Tue, 13 Feb 2018, David Feuer wrote:

> 1. Deprecate the Semigroup and Monoid instances for Data.Map.Map,
> Data.IntMap.IntMap, and Data.HashMap.HashMap in the next releases of
> containers and unordered-containers.
>
> 2. Remove the deprecated instances.
>
> 3. After another several years (four or five, perhaps?), make a major
> release of each package in which the instances are replaced with the
> following:

I like the plan. Re-adding with Semigroup superclass would prevent silent
re-interpretation of code if it is generic.
_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: Remove Semigroup and Monoid instances for Data.Map, Data.IntMap, Data.HashMap

Akio Takano
In reply to this post by David Feuer
Hi David,

On 13 February 2018 at 19:33, David Feuer <[hidden email]> wrote:
> Many people have recognized for years that the Semigroup and Monoid
> instances for Data.Map, Data.IntMap, and Data.HashMap are not so
> great. In particular, when the same key is present in both maps, they
> simply use the value from the first argument, ignoring the other one.
> This somewhat counter-intuitive behavior can lead to bugs. See, for
> example, the discussion in Tim Humphries's blog post[*]. I would like
> do do the following:

I'm against this proposal because I think the improvement is, if not
negative, too small to justify breaking existing code.

I can think of 3 reasonable definitions for (<>) for lazy maps:

1. (<>) = union
2. (<>) = unionWith (<>)
3. (<>) = Strict.unionWith (<>)

Of these, (1) is by far the most useful operation in my experience.
(2) has the advantage that it seems like the most obvious choice, but
it's not very useful in practice. (3) is slightly more useful than
(2), but feels less canonical. Also (3) seems inconsistent with how
fmap is defined.

So there doesn't seem to be a clear winner. Perhaps there shouldn't
have been a Monoid instance for maps in the first place. Given that we
already have one, however, it seems that the marginal gain of removing
it doesn't justify the cost of breaking code.

(If the removal is going to happen anyway, I vote for not adding back
any instance.)

Regards,
Takano Akio
_______________________________________________
Libraries mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
12