Added All and Any monoids #397

coot · 2024-04-26T14:20:14Z

All is a monoid build around .&&.. It is useful when writing
complex properties which check multiple conditions. Since it is
a monoid it allows one to use foldMap which is often much more
ergonomic than using conjoin.

All satisfies monoid laws up to isSuccess, unless one is using
checkCoverage & cover. I'd argue this is not a problem since
checkCoverage and cover are most often added at the top of the
property.

This patch also adds Any monoid build around .||..

Tests are also included.

MaximilianAlgehed · 2024-04-26T14:23:54Z

C.f. #279.

MaximilianAlgehed · 2024-04-26T14:35:06Z

The tricky thing here is deciding if this is the right point on the tradeoff spectrum. Maybe having Monoid Property and Any as an option is another good option? Another problem here is that the names Any and All are taken in base.

The build also fails on a bunch of older GHC versions (and Hugs) but thats not a huge deal - it should be pretty easy to address.

phadej · 2024-04-26T14:40:26Z

src/Test/QuickCheck.hs

@@ -315,8 +315,10 @@ module Test.QuickCheck
  , (.&.)
  , (.&&.)
  , conjoin
+  , All (..)


These re-exports will for sure cause pain down the line. Without re-exporting from the main module the names are less of an issue.

I agree. It's better to remove them.

coot · 2024-04-26T17:52:14Z

The tricky thing here is deciding if this is the right point on the tradeoff spectrum.

Note that All is more general than Property, as it is an existential type which can hold any Testable p => p. When I discussed this with John, we came up with this design as it gives a lot of flexibility in writing things like Any p <> label ..., or even Any p .||. Any p' etc. I am not sure how limiting p :: Property is in general (at least, I don't usually write code which is polymorphic with Testable constraint), but in other areas, e.g. for teaching purposes, people are taking advantage of it, so students do not need to write type annotations, or modify them when playing with code.

Bodigrim · 2024-04-27T13:22:16Z

src/Test/QuickCheck/Monoids.hs

+-- Note: monoid laws are satisfied up to `isSuccess` unless one is using
+-- `checkCoverage`.
+--
+data All = forall p. Testable p => All { getAll :: p }


Maybe AllOf / AnyOf or AllHold / AnyHolds etc.? To unclash with Data.Monoid.

Or maybe And and Or.

another idea: Every / Some

src/Test/QuickCheck/Monoids.hs

MaximilianAlgehed · 2024-05-02T12:15:32Z

src/Test/QuickCheck/Monoids.hs

+-- Use `property @Any` as an accessor which doesn't leak
+-- existential variables.


I don't understand what this means, can you give an example / explanation here?

Consider you have an abient function which returns Property, if you try to use getAny GHC will complain that forall p. Testable p => p is not Property, or it might error by saying that an existential p is leaking, in that scenario just using property (or property . getAny) is what one needs.

λ a = All True λ :t a a :: All λ :t getAll a <interactive>:1:1: error: [GHC-55876] • Cannot use record selector ‘getAll’ as a function due to escaped type variables • In the expression: getAll a Suggested fix: Use pattern-matching syntax instead λ :t case a of All x -> x <interactive>:1:20: error: [GHC-25897] • Couldn't match expected type ‘p’ with actual type ‘p1’ ‘p1’ is a rigid type variable bound by a pattern with constructor: All :: forall p. Testable p => p -> All, in a case alternative at <interactive>:1:11-15 ‘p’ is a rigid type variable bound by the inferred type of it :: p at <interactive>:1:1 • In the expression: x In a case alternative: All x -> x In the expression: case a of All x -> x • Relevant bindings include x :: p1 (bound at <interactive>:1:15)

but

λ :t property a property a :: Property

Ok, I see what you mean. I'm inclined to say at this point that it's better to just not say anything / point out that the types are Testable.

Also, the type of getEvery and getSome are such that they can basically never be used. I'd prefer to remove them from the code all together to keep it cleaner.

MaximilianAlgehed · 2024-05-02T12:15:45Z

src/Test/QuickCheck/Monoids.hs

+-- Use `property @All` as an accessor which doesn't leak
+-- existential variables.


I don't understand what this means, please give an example.

tests/Monoids.hs

MaximilianAlgehed · 2024-05-02T12:16:34Z

tests/Monoids.hs

+                      -- , pure $ Any (checkCoverage $ cover 100 False "" True)
+                      -- , pure $ Any (checkCoverage $ cover 100 False "" False)


Suggested change

-- , pure $ Any (checkCoverage $ cover 100 False "" True)

-- , pure $ Any (checkCoverage $ cover 100 False "" False)

Actually, when I included them, all tests pass - unlike for All / Every where the unit property fails.

coot · 2024-06-04T15:24:31Z

@MaximilianAlgehed I addressed your comments.

MaximilianAlgehed · 2024-06-04T15:26:10Z

Great! Please fix the build failures and I'll have a look next week! (OOTO this week)

coot · 2024-07-09T05:43:49Z

@MaximilianAlgehed could you approve the workflows to run?

coot · 2024-07-09T06:34:50Z

Something went terribly wrong with all the linux jobs:

Run actions/checkout@v3
/usr/bin/docker exec  f8c8e73e602a1181103553ab346548bbea1153df1ee39cd8bca903f874d0dd8f sh -c "cat /etc/*release | grep ^ID"
/__e/node20/bin/node: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.28' not found (required by /__e/node20/bin/node)

coot · 2024-07-09T06:35:53Z

Let's try to re-run them.

MaximilianAlgehed · 2024-07-09T06:48:01Z

@coot I'm working on a fix for the CI issue in #412

MaximilianAlgehed · 2024-07-24T07:28:56Z

@coot sorry for the delay but the fix has been merged

coot · 2024-07-24T16:46:46Z

I rebased this PR.

MaximilianAlgehed · 2024-07-24T17:02:16Z

Ok, sorry... You need to rebase again because I screwed up with the semantics of github actions and the difference between "run on push" and "run on PR". There is no neat way to get "run on push and also run on PR from forks"... Anway, hopefully this works for now.

coot · 2024-07-24T18:57:18Z

Done.

`All` is a monoid build around `.&&.`. It is useful when writing complex properties which check multiple conditions. Since it is a monoid it allows one to use `foldMap` which is often much more ergonomic than using `conjoin`. `All` satisfies `monoid` laws up to `isSuccess`, unless one is using `checkCoverage` & `cover`. I'd argue this is not a problem since `checkCoverage` and `cover` are most often added at the top of the property. This patch also adds `Any` monoid build around `.||.`. Tests are also included.

coot · 2024-08-22T15:31:34Z

Let's see if I fixed 7.10.3. I was getting a linking error with ghc-7.10.3 locally.

MaximilianAlgehed requested a review from UlfNorell April 26, 2024 14:35

phadej reviewed Apr 26, 2024

View reviewed changes

Bodigrim reviewed Apr 27, 2024

View reviewed changes

MaximilianAlgehed requested changes May 2, 2024

View reviewed changes

coot force-pushed the coot/monoids branch from 7ecb241 to 6bb86c3 Compare May 4, 2024 10:43

coot force-pushed the coot/monoids branch from 6bb86c3 to 7828c05 Compare July 9, 2024 05:42

coot force-pushed the coot/monoids branch from 7828c05 to 6e02a1b Compare July 9, 2024 06:35

MaximilianAlgehed mentioned this pull request Jul 9, 2024

Bump workflow versions #412

Merged

coot force-pushed the coot/monoids branch from 6e02a1b to 1f3880c Compare July 24, 2024 16:45

coot force-pushed the coot/monoids branch from 1f3880c to efffd44 Compare July 24, 2024 18:56

coot force-pushed the coot/monoids branch from efffd44 to 4017519 Compare July 25, 2024 05:16

coot force-pushed the coot/monoids branch from 4017519 to 3b6ca03 Compare August 22, 2024 14:51

GHC-7.10.3 support

d14fccf

coot force-pushed the coot/monoids branch from d63daa5 to d14fccf Compare August 22, 2024 15:28

MaximilianAlgehed approved these changes Aug 26, 2024

View reviewed changes

UlfNorell approved these changes Aug 27, 2024

View reviewed changes

MaximilianAlgehed merged commit d66336c into nick8325:master Aug 27, 2024
52 checks passed

coot deleted the coot/monoids branch August 28, 2024 06:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added All and Any monoids #397

Added All and Any monoids #397

coot commented Apr 26, 2024

MaximilianAlgehed commented Apr 26, 2024

MaximilianAlgehed commented Apr 26, 2024 •

edited

Loading

phadej Apr 26, 2024 •

edited

Loading

coot Apr 26, 2024

coot commented Apr 26, 2024

Bodigrim Apr 27, 2024

coot Apr 27, 2024

coot Apr 27, 2024

MaximilianAlgehed May 2, 2024

coot May 2, 2024

MaximilianAlgehed May 6, 2024

MaximilianAlgehed May 2, 2024

MaximilianAlgehed May 2, 2024

coot May 4, 2024

coot commented Jun 4, 2024

MaximilianAlgehed commented Jun 4, 2024 •

edited

Loading

coot commented Jul 9, 2024

coot commented Jul 9, 2024

coot commented Jul 9, 2024

MaximilianAlgehed commented Jul 9, 2024

MaximilianAlgehed commented Jul 24, 2024

coot commented Jul 24, 2024

MaximilianAlgehed commented Jul 24, 2024

coot commented Jul 24, 2024

coot commented Aug 22, 2024

		-- Use `property @Any` as an accessor which doesn't leak
		-- existential variables.

		-- Use `property @All` as an accessor which doesn't leak
		-- existential variables.

		-- , pure $ Any (checkCoverage $ cover 100 False "" True)
		-- , pure $ Any (checkCoverage $ cover 100 False "" False)

Added All and Any monoids #397

Added All and Any monoids #397

Conversation

coot commented Apr 26, 2024

MaximilianAlgehed commented Apr 26, 2024

MaximilianAlgehed commented Apr 26, 2024 • edited Loading

phadej Apr 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coot commented Apr 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coot commented Jun 4, 2024

MaximilianAlgehed commented Jun 4, 2024 • edited Loading

coot commented Jul 9, 2024

coot commented Jul 9, 2024

coot commented Jul 9, 2024

MaximilianAlgehed commented Jul 9, 2024

MaximilianAlgehed commented Jul 24, 2024

coot commented Jul 24, 2024

MaximilianAlgehed commented Jul 24, 2024

coot commented Jul 24, 2024

coot commented Aug 22, 2024

MaximilianAlgehed commented Apr 26, 2024 •

edited

Loading

phadej Apr 26, 2024 •

edited

Loading

MaximilianAlgehed commented Jun 4, 2024 •

edited

Loading