Add monoid instance for Property

[isovector]

Fixes #279

[phadej]

Fails on old GHCs: https://travis-ci.org/github/nick8325/quickcheck

[isovector]

Whatever's failing, it's not immediately due to this PR. CI is complaining about this on 8.6:

Test/QuickCheck/Random.hs:35:10: error:
    • No instance for (RandomGen SMGen) arising from a use of ‘next’
    • In the expression: next g
      In the expression: case next g of { (x, g') -> (x, QCGen g') }
      In an equation for ‘next’:
          next (QCGen g) = case next g of { (x, g') -> (x, QCGen g') }
   |
35 |     case next g of
   |          ^^^^^^

[phadej]

Even if you rebase on top of #299, it will still fail. Semigroup is fairly recent addition to base.

[isovector]

ptal

[Bodigrim]

It seems a bit opinionated to decide that (<>) on Property is (.&&.) and not (.||.). How do you use this instance BTW?

[coot]

It seems a bit opinionated to decide that (<>) on Property is (.&&.) and not (.||.).

Monoid instance based on .&&. would be quite useful and unsurprising: most often one needs to check that every property is satisfied. For Monoid based on .||. it would be better to have a newtype wrapper (if anything at all).

How do you use this instance BTW?

One can fold or foldMap over a list of properties, which is more convenient than using foldr.

[Bodigrim]

One can fold or foldMap over a list of properties, which is more convenient than using foldr.

But there is conjoin to do this unambiguously and without guessing about chosen semantics of Monoid.

[Bodigrim]

Off the top of my head, I do not expect instance Monoid Property to exist at all, because there is no instance Monoid Bool (and no instance Monoid Int, etc.). If I encounter any, I'd have to dig into QuickCheck source code to learn its exact semantics.

What exactly is the added value of instance Monoid Property? Note that existing (.&&.) and conjoin have more general types than proposed (<>) and mconcat.

[coot]

foldMap is more general than conjoin @Property, though one can use conjoin . map f instead. However, I still prefer foldMap as this is a standard pattern which one should not rediscover again.

In general there is no bias whether Bool should be monoid with respect to && or ||, hence the newtype wrappers Any and Some, but in testing conjunction is more useful / common and less surprising than disjunction.

Note that existing (.&&.) and conjoin have more general types than proposed (<>) and mconcat. Neither foldMap nor conjoin are more general than the other (through substitution).

[Bodigrim]

but in testing conjunction is more useful / common and less surprising than disjunction.

With all due respect, in my experience this is far from certain. I use .&&. on an equal footing with .||..

[isovector]

Seeing as I found #279 while wanting (<>) = (.&&.), and @coot appears to have found this PR in the same pursuit, and I have literally never written a series of properties that wanted to use (.||.), and that I'd bet good money that most people are in that boat, and that there is no issue or PR for using the disjunctive, this is clearly the right instance. Don't let some theoretical concern get in the way of doing the right thing.

[Bodigrim]

I'm sorry @isovector, but stigmatizing my concerns as "theoretical" will not make them disappear. I salute your courage to bet, but IMO such acts rarely contribute to making something "the right thing". Nevertheless, I regret that my comments provoked more heat than appropriate for a passer-by.

[nfrisby]

I opened the Issue #279 that @isovector linked, so I clearly also favor <> = && . I think there is plenty of precedent for choosing one of many possible binops as <>. (I agree with @Bodigrim that Bool is rightly not such a precedent.)

We'd need, say, a poll to persuade @Bodigrim and any who share his opinoin, I suppose. Maybe libraries@haskell.org has experience/can stand-in for that kind of thing?

Do you have a stance, @nick8325? Thanks all. Thanks @isovector for progressing the issue with a PR.