Implementation of seqBifoldable #59
Conversation
…e instance, corrected typoes
|
This PR partially fixes #26, but I don't remove seqMap. I can do that if necessary. Also, I don't know why the CI is not running. |
There was a problem hiding this comment.
This should also add tests, given that the proposed code doesn't work. However, for that, we should first make a cabal test suite (#38), which I'll hopefully get to soon.
I'd be fine with deprecating seqMap and removing it later, with a deprecation period.
| -- | Evaluate the elements of a bifoldable data structure according to | ||
| -- the given strategy | ||
| seqBifoldable :: Bifoldable p => Strategy a -> Strategy b -> Strategy (p a b) | ||
| seqBifoldable = bifoldMap |
There was a problem hiding this comment.
This doesn't work, the Monoid () instance doesn't force anything.
This should work:
| seqBifoldable = bifoldMap | |
| seqBifoldable strat1 strat2 = bifoldr (\x acc -> strat1 x `seq` acc) (\y acc -> strat2 y `seq` acc) () |
| @@ -44,6 +44,7 @@ library | |||
| build-depends: | |||
| array >= 0.3 && < 0.6, | |||
| base >= 4.3 && < 4.17, | |||
| bifunctor , | |||
There was a problem hiding this comment.
This should have a bound and only be used for GHC < 8.2 (since Bifoldable is included in base since GHC 8.2 / base-4.10). The most liberal bounds should be fine, since we only use bifoldr (at least in my suggestion).
| bifunctor , | |
| bifunctors >= 0.1 && < 5.7, |
bifunctor is a deprecated package that doesn't even export the stuff we're using.
This is based off issue #26 although I did not remove
seqMap. I can do this if desired. I also did not specify versions for the added bifunctor package dependency in parallel.cabal. I was not sure what versions should be specified as it seems the newer ones introduce additional dependencies. Please let me know how I should proceed, and I will make the necessary adjustments.Also note that the first commit has a typo. I meant Bifoldable in all instances, not Bifunctor.