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 Sequence intent. #46

Closed
wants to merge 7 commits into from

Conversation

tomprince
Copy link
Contributor

No description provided.

@radix
Copy link
Contributor

radix commented Apr 6, 2015

Hey Tom, glad to see you're still interested in Effect :)

I can see how this offers something beyond manually chaining effects -- managing the list of results is pretty tedious in Python, but I have two questions:

  1. practically, when do you want something like this?
  2. do you understand foldM? I haven't grokked it myself, but I keep getting the feeling that maybe there is some useful combination of Effects with folds that is just beyond my understanding, and this is one of the situations that trigger that feeling. I haven't yet had the time to sit down and think about it for the requisite while, though.

@radix
Copy link
Contributor

radix commented Apr 6, 2015

@tomprince
Copy link
Contributor Author

  1. For the use case I have ([FLOC-1633] Switch provisioning to effect. ClusterHQ/flocker#1267) I don't actually need the result. But I just need to run a bunch of things (in this case, remote SSH commands) in sequence.
  2. Well, this is just sequence. Haskell's foldl is just python's reduce; foldM is just the case where the reducing function has side-effects. I think sequence can be expressed in terms of foldM
sequence = foldM (\x y -> return (x ++ [y])) []

but I don't think that that is simpler.

@radix radix mentioned this pull request May 7, 2015
@radix
Copy link
Contributor

radix commented May 7, 2015

@tomprince

Could you check out #51 and #52?

I think this is a nicer formulation. Basically I just split up the "folding over effects" part of your PR from the "accumulating into a list" part. The error-handling is generalized to all folds, such that it raises a FoldError with the accumulator-so-far (not necessarily a list) and the original exception. I've also kept the optimization of appending to a list instead of concatenating lists.

I also dropped the intent. I can't think of a good reason to have an intent for it because tests would still just need to assert that each effect is performed in turn (usually with SequenceDispatcher).

@radix
Copy link
Contributor

radix commented Jun 2, 2015

closing since #52 was merged. Thanks for the contribution.

@radix radix closed this Jun 2, 2015
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.

None yet

2 participants