Remove dependency on kotlinx.coroutines for effects #250

pakoito · 2017-09-03T17:20:05Z

This diff fixes a bunch of small issues related to coroutines:

1- New constructor for EQ
2- Remove bindingInContext temporarily until we work on a non-blocking solution
3- Remove JobKW, DeferredKW and other effects-kotlinx
4- Remove all dependencies on effects-kotlinx except for testing
5- Add support for CoroutineContext on Comonad

====

ORIGINAL POST (OUTDATED)

More fixes towards making effects stable:

1.- Remove dependency on kotlinx.coroutines. This is a helper library that'll be JVM-bound.
2.- New module effects-kotlinX for DeferredKW
2.- Remove bindingInContext. It didn't work unless blocking.
3.- Add support for CoroutineContext on Comonad

Depends on #246

…s. Add deriving for instances.

…ss derivation.

codecov-io · 2017-09-03T17:47:51Z

Codecov Report

❗ No coverage uploaded for pull request base (master@b9bc7d8). Click here to learn what that means.
The diff coverage is 68.75%.

@@            Coverage Diff            @@
##             master     #250   +/-   ##
=========================================
  Coverage          ?   45.07%           
  Complexity        ?      318           
=========================================
  Files             ?      148           
  Lines             ?     3723           
  Branches          ?      402           
=========================================
  Hits              ?     1678           
  Misses            ?     1924           
  Partials          ?      121

Impacted Files	Coverage Δ	Complexity Δ
...t/src/main/kotlin/kategory/laws/MonadWriterLaws.kt	`21.42% <ø> (ø)`	`2 <0> (?)`
.../src/main/kotlin/kategory/generators/Generators.kt	`57.14% <ø> (ø)`	`0 <0> (?)`
...core/src/main/kotlin/kategory/typeclasses/Monad.kt	`72.54% <ø> (ø)`	`0 <0> (?)`
...ry-core/src/main/kotlin/kategory/typeclasses/Eq.kt	`36.36% <0%> (ø)`	`0 <0> (?)`
...ry-test/src/main/kotlin/kategory/laws/MonadLaws.kt	`1.72% <0%> (ø)`	`2 <0> (?)`
...test/src/main/kotlin/kategory/laws/TraverseLaws.kt	`8.57% <0%> (ø)`	`1 <0> (?)`
...ry-optics/src/main/kotlin/kategory/optics/Prism.kt	`70% <100%> (ø)`	`12 <2> (?)`
...ffects/src/main/kotlin/kategory/effects/data/IO.kt	`76% <100%> (ø)`	`10 <1> (?)`
...egory-core/src/main/kotlin/kategory/data/Option.kt	`56.25% <100%> (ø)`	`7 <0> (?)`
...re/src/main/kotlin/kategory/typeclasses/Comonad.kt	`65% <100%> (ø)`	`0 <0> (?)`
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9bc7d8...365a6ac. Read the comment docs.

raulraja · 2017-09-13T13:08:16Z

kategory-effects-kotlinx/src/main/kotlin/kategory/effects/data/DeferredKW.kt

+typealias DeferredResult<A> = Either<Throwable, A>
+
+@higherkind
+@deriving(Monad::class, AsyncContext::class)


Should

@deriving(Functor::class, Applicative::class, Monad::class, AsyncContext::class)

raulraja · 2017-09-13T13:10:25Z

kategory-effects-kotlinx/src/main/kotlin/kategory/effects/data/DeferredKW.kt

+        fun <A, B> tailRecM(a: A, f: (A) -> DeferredKWKind<Either<A, B>>): DeferredKW<B> {
+            tailrec fun go(coroutineContext: CoroutineContext, a: A, f: (A) -> DeferredKWKind<Either<A, B>>): DeferredKW<B> {
+                val result: DeferredResult<Either<A, B>> = f(a).attempt(coroutineContext)
+                /* FIXME(paco): KT-20075 If you remove return here tailrec stops working. Jetbrains Please. */


I think that makes sense because otherwise you would not know what the stop condition in the translated loop would be. what it would be good to have by default in Kotlin is to have the last statement ina block always be the return value. Explicit returns suck.

raulraja · 2017-09-13T13:12:48Z

kategory-effects-kotlinx/src/main/kotlin/kategory/effects/data/DeferredKW.kt

+
+    fun <B> flatMap(f: (A) -> DeferredKWKind<B>): DeferredKW<B> =
+            DeferredKW { context: CoroutineContext ->
+                this@DeferredKW.attempt(context).fold(


I think flatMap with attempt defeats the purpose of having this datatype. I recall reimplementing this in an fully async way. But if we cause to block what good does deffer provides over async/await?

raulraja · 2017-09-13T13:23:53Z

kategory-effects-kotlinx/src/main/kotlin/kategory/effects/data/DeferredKW.kt

+
+@higherkind
+@deriving(Monad::class, AsyncContext::class)
+data class DeferredKW<out A>(val thunk: (CoroutineContext) -> Deferred<A>) : DeferredKWKind<A> {


I would just not consider this until we have a non-blocking solution

raulraja

This is good to go if we get rid of DeferredKW until we find a better way to implement it. 👏

# Conflicts: # kategory-core/src/test/kotlin/kategory/data/EitherTest.kt # kategory-core/src/test/kotlin/kategory/data/Function1Test.kt # kategory-core/src/test/kotlin/kategory/data/OptionTTest.kt # kategory-core/src/test/kotlin/kategory/data/OptionTest.kt # kategory-core/src/test/kotlin/kategory/free/CoYonedaTest.kt # kategory-effects/build.gradle # kategory-effects/src/main/kotlin/kategory/effects/data/IO.kt # kategory-effects/src/test/kotlin/kategory/effects/data/IOTest.kt # kategory/build.gradle # kategory/src/test/kotlin/kategory/data/CoproductTest.kt # kategory/src/test/kotlin/kategory/data/StateTTests.kt

* Rename circularBuffer to sliding, and adjust to Kotlin familiar semantics * Add Queue.dropping * Remove unused import QueueTest * Queue.dropping kdoc

pakoito added 23 commits September 2, 2017 18:12

Add initial implementation

736e092

Merge remote-tracking branch 'origin/master' into paco-deferredkw

16c54af

Fix unsafeRun

9a057d9

Fix tests

3319e4b

Move logged EQ to an extension function available only on the spec

130e190

Simplify Eq with a new constructor

e89f282

Remove redundant parameters

c492533

Update documentation

5980bec

Nit

5f2439d

Fix async and tailrecM for DeferredKW

a940a90

Fix async and tailrecM for DeferredKW

7ff893f

Add a proper fixme

17a0754

Fix detekt

9f0aa0d

Remove map/flatMap dependency on CorutineContext

759dabe

Fix tailrecM blocking the current thread

5d9a571

Refactor DeferredKW to use CoroutineContext only when execution start…

94bbfa0

…s. Add deriving for instances.

Remove redundant instance helpers

4ed29e0

Move CoroutineContext to be required only for evaluation. Add typecla…

6219f67

…ss derivation.

Remove JobKW

a998b8f

Move kotlinx.coroutines to its own module

534098f

Remove dependencies on kotlinx.coroutines

acaf2d6

Remove bindingInContext. It didn't quite work unless it blocked.

4bb9864

Comonad cobinding now allows setting its corutine context

5f3ae26

pakoito requested review from raulraja and a team September 3, 2017 17:20

Fix detekt

7590a59

raulraja requested changes Sep 13, 2017

View reviewed changes

raulraja approved these changes Sep 13, 2017

View reviewed changes

Remove kategory-effects-kotlinx

114929c

pakoito added 3 commits September 13, 2017 20:39

Fix merge conflicts

dc2a9ff

Fix detekt

365a6ac

raulraja approved these changes Sep 13, 2017

View reviewed changes

pakoito merged commit 9c81e7a into master Sep 13, 2017

pakoito deleted the paco-effectsfix branch September 13, 2017 22:03

wiyarmir mentioned this pull request Sep 15, 2017

Release 0.3.6 #279

Closed

rachelcarmena pushed a commit that referenced this pull request Feb 24, 2021

Implement Either#merge() (#250)

1f1a1c8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove dependency on kotlinx.coroutines for effects #250

Remove dependency on kotlinx.coroutines for effects #250

pakoito commented Sep 3, 2017 •

edited

Loading

codecov-io commented Sep 3, 2017 •

edited

Loading

raulraja Sep 13, 2017

raulraja Sep 13, 2017

raulraja Sep 13, 2017 •

edited

Loading

raulraja Sep 13, 2017

raulraja left a comment

Remove dependency on kotlinx.coroutines for effects #250

Remove dependency on kotlinx.coroutines for effects #250

Conversation

pakoito commented Sep 3, 2017 • edited Loading

codecov-io commented Sep 3, 2017 • edited Loading

Codecov Report

raulraja Sep 13, 2017

Choose a reason for hiding this comment

raulraja Sep 13, 2017

Choose a reason for hiding this comment

raulraja Sep 13, 2017 • edited Loading

Choose a reason for hiding this comment

raulraja Sep 13, 2017

Choose a reason for hiding this comment

raulraja left a comment

Choose a reason for hiding this comment

pakoito commented Sep 3, 2017 •

edited

Loading

codecov-io commented Sep 3, 2017 •

edited

Loading

raulraja Sep 13, 2017 •

edited

Loading