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

add ListT monad transformer and relevant instances #71

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

chessai
Copy link
Member

@chessai chessai commented Oct 3, 2018

Relevant issue: #54

Don't merge yet, some unresolved questions:

  1. Should we include the Enumerator part of the ListT API from pipes? I have not done so here.
  2. Should we incur a dependency on exceptions to provide MonadCatch and MonadThrow instances for ListT? There seems to already be the desire to incur this dependency on exceptions in issue [Discussion] Should we bring back exceptions instances? #52.
  3. Should the name of this be ListT, or should it be something else?

@treeowl
Copy link
Contributor

treeowl commented Oct 3, 2018

If we add this, it definitely should be in its own module!

@chessai
Copy link
Member Author

chessai commented Oct 3, 2018

If we add this, it definitely should be in its own module!

Oh yeah, that was another question I had, but forgot to ask in the first comment. I think I agree.

tell w = lift (tell w)
{-# INLINE tell #-}

--listen :: ListT m a -> ListT m (a, w)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's all this do?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

define "all this"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just mean it's not obvious to me what listen has to do in a stream contact and why this is how it's done. I'd appreciate a comment.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially I thought that MonadWriter might be useful to build up a ListT, and then consume it, but I think an actual use of this might be misguided. Composing Writer with ListT might (would?) be bad; Writer pretty much guarantees space leaks, and ListT is meant for streaming. If someone is ever using the MonadWriter instance for something like ListT, they are likely to be doing something poorly.

Not sure if this instance should exist anymore.

pure (go p' $! mappend w w') )
Step (a :> rest) -> Step ( (a,w) :> go rest w)

pass l = Select (go (enumerate l) mempty)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this?

@treeowl
Copy link
Contributor

treeowl commented Oct 3, 2018

Don't forget to revert all the Streaming.Prelude changes.

@chessai
Copy link
Member Author

chessai commented Oct 3, 2018

I thought I did?

@treeowl
Copy link
Contributor

treeowl commented Oct 3, 2018 via email

@PierreR
Copy link
Contributor

PierreR commented Oct 5, 2018

@chessai
Copy link
Member Author

chessai commented Oct 10, 2018

What about changing the README ?
https://github.com/haskell-streaming/streaming#-5-how-come-theres-not-one-of-those-fancy-listt-done-right-implementations-in-here

Yes, if this is approved, the README change will be the last commit to occur. Thanks for pointing this out.

@danidiaz
Copy link
Contributor

danidiaz commented Oct 13, 2018

Should we incur a dependency on exceptions to provide MonadCatch and MonadThrow instances for ListT?

I'm against depending on "exceptions" in the main "streaming" package. I would prefer the package to remain agnostic about exception and resource handling, even at the cost of some inconvenience.

@chessai
Copy link
Member Author

chessai commented Oct 22, 2018

Should we incur a dependency on exceptions to provide MonadCatch and MonadThrow instances for ListT?

I'm against depending on "exceptions" in the main "streaming" package. I would prefer the package to remain agnostic about exception and resource handling, even at the cost of some inconvenience.

Understandable, it would be nice if more people weighed in on this.

@haskell-streaming haskell-streaming deleted a comment from chessai Oct 22, 2018
@haskell-streaming haskell-streaming deleted a comment from chessai Oct 22, 2018
@haskell-streaming haskell-streaming deleted a comment from chessai Oct 22, 2018
@haskell-streaming haskell-streaming deleted a comment from chessai Oct 22, 2018
@haskell-streaming haskell-streaming deleted a comment from chessai Oct 22, 2018
@haskell-streaming haskell-streaming deleted a comment from chessai Oct 22, 2018
@haskell-streaming haskell-streaming deleted a comment from chessai Oct 22, 2018
@chessai
Copy link
Member Author

chessai commented Oct 22, 2018

I apologise for the spam - last night's outage caused GitHub to report that i was unable to comment, i hit the comment button multiple times. I guess after things got better, all copies of the comment went through.

@masaeedu
Copy link

Are the unresolved questions still unresolved?

@chessai
Copy link
Member Author

chessai commented May 25, 2020

My current personal answer to the questions is No, No, and No. Maybe others have thoughts?

@danidiaz
Copy link
Contributor

I agree with the triple-no.

The ListT from "pipes" has a Monoid and compatible Alternative and MonadPlus instances that perform concatenation. Perhaps they could be added here too?

@danidiaz
Copy link
Contributor

danidiaz commented Jun 15, 2020

This 2014 post by Gabriel Gonzalez gives examples of how concatenation-based MonadPlus instances can be used to write simple library-agnostic streaming sources.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants