-
Notifications
You must be signed in to change notification settings - Fork 11
Remove Monad
and Applicative
constraints on the Parallel
class
#43
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
Conversation
src/Control/Parallel/Class.purs
Outdated
parallel :: m ~> f | ||
sequential :: f ~> m | ||
|
||
instance monadParExceptT :: Parallel f m => Parallel (Compose f (Either e)) (ExceptT e m) where | ||
instance monadParExceptT :: (Parallel f m, Apply (ExceptT e m)) => Parallel (Compose f (Either e)) (ExceptT e m) where |
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.
These constraint changes don't seem right, ExceptT e m
is already Apply
if m
is Monad
: https://github.com/purescript/purescript-transformers/blob/be72ab52357d9a665cbf93d73ba1c07c4b0957ee/src/Control/Monad/Except/Trans.purs#L58
Same goes for MaybeT
.
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.
But m isn't a Monad anymore, that's the point of the pr, and that's why i had to add these constraints
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.
Oh sorry, I misread the diff. In that case, the constraint here should be Monad m
. It's impossible for there to be an Apply (ExceptT e m)
instance otherwise anyway.
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.
Do you have an example of a case where this new formulation is useful, by the way?
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 constraint here should be Monad m. It's impossible for there to be an Apply (ExceptT e m) instance otherwise anyway.
That's a good point, i didn't think of that. I thought Monad m
would be more restrictive, but i see how in this function, it isn't. I agree that Monad m
is clearer and better, in that case.
Do you have an example of a case where this new formulation is useful, by the way?
My use case is that i want to try making a parser that is applicative, but not a monad, so that its structure can be observed without actually running it on something (and it'll be built up from a symbolic representation, kinda like explained towards the end of this post: https://cronokirby.com/posts/2020/10/lexical-combinators/).
Then i thought i could have a newtype around that, which instead composes parsers in a way that lets them parse in any order (for example, parsing command line arguments). This would be a good candidate for a Parallel
instance, i thought. You'd be able to use parApply
(or Par.ado
) to make permutable sub-parsers in an overall sequential parser.
Basically i think the "parse these applicatively combined parsers in any order" is a good parallel analogy to "parse these applicatively combined parsers in the order they were combined", and there are reasons to make sequential parsers that are not monads.
I think this looks good. I wonder if a compromise would to still require But I would guess most people are probably using |
To provide more context for the motivation, say you have a parser applicative (not monad, because being only applicative lets it be statically analyzed, i.e. you can interpret a free applicative while ignoring the result type of every intermediate calculation, something you can not do with a free monad). the parser it would be useful to give this a |
The potential breaking changes would only be for functions that are general over The reason i went for |
Description of the change
Both types now only require
Apply
. This is to allow more types to have aParallel
instance.Closes #24
Checklist: