Add Apply.map2Eval and allow traverse laziness #1015
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes typelevel#513. I think this should solve the majority of use-cases for lazy traversal. One thing I noticed is that both `reduceRightToOption` and `traverse1_` (which uses the former) seem to be impossible to implement lazily such that they allow for short-circuiting. At least I couldn't figure out a way. I left a note in the `traverse1_` scaladoc, but in practice I don't anticipate this being a problem, because at the cost of just having an `Applicative` instance as opposed to `Apply`, you can use `traverse_` which is sufficiently lazy.
|
Another note: it kind of seems like there should be things like |
|
@julien-truffaut would you want to review this since you originally opened up #513? |
|
👍 |
| foldLeft(fa, G.pure(())) { (acc, a) => | ||
| G.map2(acc, f(a)) { (_, _) => () } | ||
| } | ||
| foldRight(fa, Later(G.pure(()))) { (a, acc) => |
There was a problem hiding this comment.
You might want to use Always here, since it will be evaluated once and there's no need for memoization.
This avoids a bit of memoization overhead in places that we only expect to be evaluated once.
Current coverage is 82.23%
@@ master #1015 diff @@
==========================================
Files 215 215
Lines 2704 2713 +9
Methods 2639 2647 +8
Messages 0 0
Branches 60 61 +1
==========================================
+ Hits 2217 2231 +14
+ Misses 487 482 -5
Partials 0 0
|
|
👍 |
1 similar comment
|
👍 |
ceedubs
added a commit
to ceedubs/cats
that referenced
this pull request
Dec 10, 2017
This adds a test case that the `traverse` implementation for `Vector` short-circuits the traversal when possible. This would have failed the build in typelevel#2091. See typelevel#1015 for when similar tests were added for similar structures.
johnynek
pushed a commit
that referenced
this pull request
Dec 10, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #513.
I think this should solve the majority of use-cases for lazy traversal.
One thing I noticed is that both
reduceRightToOptionandtraverse1_(which uses the former) seem to be impossible to implement lazily such
that they allow for short-circuiting. At least I couldn't figure out a
way. I left a note in the
traverse1_scaladoc, but in practice I don'tanticipate this being a problem, because at the cost of just having an
Applicativeinstance as opposed toApply, you can usetraverse_which is sufficiently lazy.