Incompatible 'intercalate' based on the version of mono-traversable

[hdgarrood]

Looking at the source, it seems that if you have mono-traversable < 0.9.3, you get intercalate with this type:

(Monoid (Element c), IsSequence c) => Element c -> c -> Element c

But if you have mono-traversable >= 0.9.3, you get the one from Data.Sequences, whose type is:

SemiSequence seq => seq -> [seq] -> seq

These seem to be incompatible. Is this on purpose? The second definition was not as good a fit for my specific case, because I wanted intercalate :: Html -> [Html] -> Html, and Html is a Monoid but not a SemiSequence. I noticed this while upgrading dependencies; intercalate was previously instantiated at that type for the older version, but it now cannot be.

In case it's of interest, what I've ended up doing now is hiding the import of intercalate from ClassyPrelude in my application's prelude, and defining it as follows:

intercalate :: (Monoid (Element seq), SemiSequence seq) =>
  Element seq -> seq -> Element seq
intercalate x xs = concat (intersperse x xs)

[snoyberg]

This looks like it was an accidental regression. Want to send a PR to get back the original behavior? Or better yet, can the intercalate in mono-traversable be generalized?

[hdgarrood]

I'd happily send a PR, I'm just not sure how to change it yet :)

As far as I can tell, the option defined in terms of concat and intersperse seems most appropriate. I also think that most uses of the current intercalate in mono-traversable would continue to work if we were to change it like that, as SemiSequence implies Monoid, and [] has an IsSequence instance; arguably it's just a generalisation.

I guess the question is, then, do we want to define intercalate as a member of SemiSequence, or outside the class, as I have above?

[snoyberg]

Looking into this a bit further:

1. The original move of making the API conditional was definitely a mistake
2. There is good reason for the restrictive type signature in mono-traversable, namely: there are optimized versions for many types when constrained to lists
3. Nothing we've provided so far is the most general type signature

I think our best bet would be to leave mono-traversable as-is, and in classy-prelude, define an intercalate function with the general type signature, regardless of the version of mono-traversable used. When we have mono-traversable 0.9.3 and up, we can define rewrite rules that replace optimized versions of the function when appropriate.

So what's the most general signature? One of the following, depending on whether we prefer Semigroup or Monoid as being more general:

intercalate :: (MonoFoldable mono, Semigroup (Element mono)) => Element mono -> mono -> Element mono
intercalate :: (MonoFoldable mono, Monoid (Element mono)) => Element mono -> mono -> Element mono

I lean towards defining this in terms of Semigroup, since Semigroup will be moving to base soon and eventually become a superclass of Monoid (as it is mathematically). However, I think we should keep it at Monoid for now to avoid a major version bump.

Does this make sense?

And any thoughts @gregwebs?

[hdgarrood]

:+1: sounds good to me. The only thing is, I think we might need Monoid (Element Mono), as I would expect intercalate x xs == mempty when onull xs holds, regardless of the value of x.

[snoyberg]

Good point

On Tue, Feb 23, 2016 at 5:38 PM, Harry Garrood notifications@github.com wrote:

[image: :+1:] sounds good to me. The only thing is, I think we might need Monoid (Element Mono), as I would expect intercalate x xs == mempty when onull xs holds, regardless of the value of x.

— Reply to this email directly or view it on GitHub https://github.com/snoyberg/classy-prelude/issues/118#issuecomment-187751661 .