search

snowleopard / alga

Algebraic graphs
MIT License
717 stars 68 forks source link

Fix compatibility with QuickCheck 2.14.2 #266

Closed felixonmars closed 3 years ago

felixonmars commented 3 years ago

Fixes #265

snowleopard commented 3 years ago

@felixonmars Thanks for the issue and the fix!

It's likely that the custom Arbitrary Tree instance defined here and the instance provided by quickcheck-2.14.2 are different. Did you investigate the differences? If they are indeed substantially different, then swapping one for the other depending on which QuickCheck version happens to be found by the constraint solver doesn't seem right. If they are more or less the same, then it's probably OK.

felixonmars commented 3 years ago

https://github.com/nick8325/quickcheck/blob/3c098d757a04006d1b1f127c5a1789f270d70547/src/Test/QuickCheck/Arbitrary.hs#L825

They do look different, but the tests are passing here. I am not an expert here, though :(

snowleopard commented 3 years ago

@felixonmars I decided not to overcomplicate things and merged the PR. Thank you!

(P.S.: A better approach could have been to stick to the existing instance, but wrap Tree into a newtype and unwrap it in the corresponding tests. But that seems like way too much work for little benefit.)