Repository navigation
Add definitions for stimes - #340
Conversation
Remove unused `LambdaCase` extension
There was a problem hiding this comment.
Thanks!
The stimes definitions look good, but I'm less sure with sconcat and mconcat.
For these I think it would be best to have some tests to check the Semigroup and Monoid laws. We can probably use checkers or quickcheck-classes for this. (The transitive semigroupoids dependency may need to be configured with -f-unordered-containers.)
The performance implications also are unclear to me. I think it would be best to resolve #139 before using unions in mconcat and sconcat.
Do you want to work on this? If you don't (which would be perfectly fine!), I suggest you remove the changes to mconcat and sconcat for now.
|
@sjakobi it all looks fine to me. I agree tests would be good (for |
Are you referring to the
Good point, I'll remove the |
|
I'm just saying that we have to be careful not to change the current semantics (whatever they are) unintentionally. |
mconcat, sconcat, stimesstimes
I was referring to that law and the other ones documented in |
|
So the semantic change resulting from the explicit instead of before I'd be comfortable with including this in a minor release. |
|
Yeah, a semantic improvement is totally fine. I'm talking about the left vs. right bias in |
I was wrong, it doesn't. If we just define |
|
@konsumlamm I imagine that we could define a |
|
Thank you, @konsumlamm! :) |
Resolves part of #307.
I added
INLINEpragmas, for consistency with the other definitions, although they're probably not needed.Also removed two unused
{-# LANGUAGE LambdaCase #-}.EDIT: I originally also added definitions for
mconcatandsconcat, but removed them again (see discussion below).