-
Notifications
You must be signed in to change notification settings - Fork 483
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix implementation of isEmpty and overlaps in Interval.hs #3042
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation looks ok now, thank you.
You could add some property tests for this to plutus-ledger/test/Spec.hs
as well. For example, every non-empty interval overlaps with itself. There are some examples already.
@@ -225,7 +225,7 @@ member a i = i `contains` singleton a | |||
-- | Check whether two intervals overlap, that is, whether there is a value that | |||
-- is a member of both intervals. | |||
overlaps :: Ord a => Interval a -> Interval a -> Bool | |||
overlaps l r = isEmpty (l `intersection` r) | |||
overlaps l r = not $ isEmpty (l `intersection` r) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops!
Sure, I already started implementing basic test cases, e.g. Am currently familiarizing myself more with Haskell and (property based) testing in Haskell before daring to submit those. Thanks for the example and pointing me in the direction! |
fd446cb
to
398dab9
Compare
I added a couple of test cases covering Sadly, the Hydra CI check fails, but it seems to be failing in |
Flaky VM test, restarted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks for adding the tests.
A fellow Plutus Pioneer Program student (BC Chuah) noticed an odd behavior of the
overlaps
function.Upon further investigation, I noticed that both
overlaps
andisEmpty
needed a change to work as expected.Pre-submit checklist:
Pre-merge checklist: