Replace parTupled with parZipOrAccumulate #2957
Conversation
Kover Report
|
| crossinline fa: suspend ScopedRaise<E>.() -> A, | ||
| crossinline fb: suspend ScopedRaise<E>.() -> B, |
There was a problem hiding this comment.
This can still benefit from the ScopedAccumulatingRaise type from #2952, and makes me wonder if Either.zipOrAccumulate shouldn't be DSL based as well to support both E and Nel<E> in a single API. 🤔
Currently we support both Either and EitherNel separately, and they both types don't interop with each-other.
|
Thanks, @nomisRev. I'm happy to continue working on this, using this branch. |
nomisRev
force-pushed
the
sv-parZipOrAccumulate
branch
from
aabef0f
to
b7a817a
Compare
nomisRev
force-pushed
the
sv-parZipOrAccumulate
branch
from
35366a6
to
445891b
Compare
2- to 9-arity based on SV's pattern for 2-arity.
|
@serras I just need to add docs and Knit examples. Do my code changes look okay? |
There was a problem hiding this comment.
This looks great! Thank you so much @lgtout 🙌 🥳
I think we can move ahead and merge this already, you can add a document with Knit examples later-on. Or perhaps directly in the new website. WDYT @serras? I cannot approve my own PR, but I've upgraded it to ready-to-review and tagged some reviewers
|
@nomisRev Ok. Sounds good regarding adding docs and examples later on. |
|
@nomisRev I'm working on the docs right now so we don't forget. I've already approved, we need somebody else from the team to give the green check. |
|
@lgtout it's been added directly in the new website. So I don't think we need elaborate documentation on this API here. Perhaps linking to I am merging this PR already, a PR adding minimal documentation is welcome but not blocking for the RC release. |
Including a reference to the parallel versions as requested in arrow-kt/arrow#2957 --------- Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com>
This week I had a discussion with @lgtout about
parTupled. We were originally on the fence to addparTupledin #2900 due to signature conflicts withparZip.Whilst writing some examples, and documentation I encountered issues with
parTupledwhere the current signatures are also not valid. It cannot be resolved without explicitely specifyingcontext: CoroutineContextparameter. After some discussion with @lgtout on Kotlin Slack, https://kotlinlang.slack.com/archives/C8UK6RTHU/p1677616555972749 we came to a mutual conclusion it's maybe not worth adding it since it requires an additional 9 methods. Total of 18 methods to provide something that practically already exists inparZip.Whilst working on some other documentation about accumulating parallel errors I realised that we only provide an API for this over
Iterablebut not zipping independent operations.This PR proposes a small convenience API to cover that use-case.
@lgtout If everyone agrees on this API are you interested in continuing this work? I only played out the arity-2 signatures, and you're already familiar with the layout of Arrow Fx Coroutines +☺️
parZipso I thought this might be a good ticket for you to work on if you're interested. I can close this PR, or feel free to work on this branch directly. Whatever would work better for you