Skip to content
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

Prepare for 2.0 release #77

Merged
merged 10 commits into from
Oct 10, 2016
Merged

Prepare for 2.0 release #77

merged 10 commits into from
Oct 10, 2016

Conversation

garyb
Copy link
Member

@garyb garyb commented Oct 10, 2016

Resolves #72, resolves #63, resolves #60

Still to do:

@garyb
Copy link
Member Author

garyb commented Oct 10, 2016

I'll finish up tomorrow, unless anyone beats me to it.

@garyb
Copy link
Member Author

garyb commented Oct 10, 2016

As far as I can tell, ExceptT needs to have Apply implemented as ap so that the behaviour is consistent with bind, but this does mean that we have a Monad m constraint from Apply downwards now... but, I've just realised that the change I made to MaybeT to loosen the constraints suffers from the very problem I just fixed for ExceptT.

I'm not sure what to suggest, since it seems like this the right thing, since the difference is this: the m is run regardless of whether the value is Nothing / Left in the apply case, but not in the Monad case. Maybe there's some way to make bind behave the same as apply instead, but I'm not really sure what that means, and it might have negative consequences.

Without the looser constraints we can't have MonadPar instances for MaybeT and now ExceptT it would seem, but then maybe that is correct, since they "don't work properly" in parallel?

@garyb
Copy link
Member Author

garyb commented Oct 10, 2016

Maybe the answer is to use the Monad constrained versions and provide custom applicative newtyped versions for MonadPar?

@paf31
Copy link
Contributor

paf31 commented Oct 10, 2016

Does it make sense to break up ComonadEnv and ComonadTraced too? What are examples of monads which have ask but not local etc.?

@paf31
Copy link
Contributor

paf31 commented Oct 10, 2016

Why not

apply (ExceptT f) (ExceptT x) = ExceptT (lift2 ($) <$> f <*> x)

?

Do you mean they're not equal in terms of what gets evaluated? True, but they're observationally equivalent, right?

@garyb
Copy link
Member Author

garyb commented Oct 10, 2016

That is how I changed apply for MaybeT, and how it was for EitherT - that exact implementation is what #60 complains about.

Honestly I have no idea, I stared at it for quite a long time trying to figure out whether the extra m running mattered or not.

@paf31
Copy link
Contributor

paf31 commented Oct 10, 2016

What do you mean when you say the m is run?

@garyb
Copy link
Member Author

garyb commented Oct 10, 2016

I mean, with the apply version the effects occur for both sides regardless of whether the left fails or succeeds... right?

I was going to put something together to try and prove whether it matters or not, but haven't gotten around to it yet.

@garyb
Copy link
Member Author

garyb commented Oct 10, 2016

(Trying now)

@paf31
Copy link
Contributor

paf31 commented Oct 10, 2016

A good test would be something like forever in the MaybeT (Eff _) monad, I think.

Edit: I mean replicateA, sorry.

@paf31
Copy link
Contributor

paf31 commented Oct 10, 2016

It sounds like there is an issue, but we can probably fix it with a newtype, either here (preferable) or in parallel. I would really like to be able to implement MonadPar for something related to ExceptT and MaybeT.

@garyb
Copy link
Member Author

garyb commented Oct 10, 2016

main :: Eff (console :: CONSOLE) Unit
main = do
  void $ runMaybeT $ f <*> a
  log "---"
  void $ runMaybeT $ ap f a

f :: forall a. MaybeT (Eff (console :: CONSOLE)) (a -> a)
f = do
  lift $ log "Hello"
  MaybeT $ pure Nothing

a :: MaybeT (Eff (console :: CONSOLE)) String
a = do
  lift $ log "LAUNCH THE MISSILES!"
  pure "foo"
Hello
LAUNCH THE MISSILES!
---
Hello

@garyb
Copy link
Member Author

garyb commented Oct 10, 2016

We can definitely define newtype(s) with the desired applicative behaviour, with the caveat that they'll behave in the above way. I think the example above settles the discussion over whether the ap approach is right or not here though, unfortunately.

@paf31
Copy link
Contributor

paf31 commented Oct 10, 2016

Sounds good

@garyb
Copy link
Member Author

garyb commented Oct 10, 2016

I'd vote for putting those newtypes in Parallel, as they're probably not going to be that useful on the whole outside of that situation. Given that, I think this PR is done sans any further comments.

@paf31
Copy link
Contributor

paf31 commented Oct 10, 2016

I'm fine with putting them in parallel. Can we please add ComonadTell though? It will be necessary to break things up in my pairings and day libraries when I implement a MonadTell instance for Co w.

@garyb
Copy link
Member Author

garyb commented Oct 10, 2016

That's supposed to come out of ComonadTraced right? It might need some refactoring if so, as it's already only made up of one function: track :: forall a. t -> w a -> a.

@paf31
Copy link
Contributor

paf31 commented Oct 10, 2016

Ah ok, interesting. Sounds good then.

@garyb garyb merged commit f26bbd4 into master Oct 10, 2016
@garyb garyb deleted the bump branch October 10, 2016 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants