Add TraverseFilter #255

tonilopezmr · 2017-09-05T13:58:26Z

Fixes: #253

Added TraverseFilter and TraverseFilterLaws
Added OptionTTraverseFilter, ComposedTraverseFilter
Added TraverseFilter and TraverseFilterLaws for OptionT, Option and Const

raulraja · 2017-09-05T14:04:38Z

kategory-test/src/main/kotlin/kategory/laws/TraverseFilterLaws.kt

+                    Law("TraverseFilter Laws: Identity", { identityTraverseFilter(TF, GA, cf, EQ) })
+            )
+
+    //def traverseIdentity[A, B](fa: F[A], f: A => B): IsEq[F[B]] = {


In the latest commits this is already removed 👍

raulraja · 2017-09-05T14:04:46Z

kategory-test/src/main/kotlin/kategory/laws/TraverseFilterLaws.kt

+    //}
+
+
+    // external laws:


raulraja · 2017-09-05T18:17:11Z

kategory-test/src/main/kotlin/kategory/laws/TraverseFilterLaws.kt

+object TraverseFilterLaws {
+
+    inline fun <reified F> laws(TF: TraverseFilter<F> = traverseFilter<F>(), GA: Applicative<F> = applicative<F>(), crossinline cf: (Int) -> HK<F, Int>, EQ: Eq<HK<F, Int>>): List<Law> =
+            TraverseLaws.laws(TF, GA, cf, EQ) + FunctorLaws.laws(GA, cf, EQ) + listOf(


The traverse laws already include the functor laws so no need to add them here again or tests will run twice

raulraja · 2017-09-05T18:20:53Z

kategory/src/main/kotlin/kategory/instances/ConstInstances.kt

@@ -16,6 +17,9 @@ interface ConstInstances<A> :

    override fun <T, U> foldR(fa: HK<ConstKindPartial<A>, T>, lb: Eval<U>, f: (T, Eval<U>) -> Eval<U>): Eval<U> = lb

+    override fun <G, T, U> traverseFilter(fa: HK<ConstKindPartial<A>, T>, f: (T) -> HK<G, Option<U>>, GA: Applicative<G>):
+            HK<G, HK<ConstKindPartial<A>, U>> = fa.ev().traverseFilter(f, GA)


while method args is ok to take the most generic representation of the HK or Kind aliases in return type we should return the most concrete compatible In this case HK<ConstKindPartial<A>, U> can just be Const<A, U> same for all other instance methods, return always the most concrete type so users don't have to use ev() to extract values from the HK representation.

I'm going to change also in traverse method 👍

codecov-io · 2017-09-05T18:34:04Z

Codecov Report

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

@@            Coverage Diff            @@
##             master     #255   +/-   ##
=========================================
  Coverage          ?   43.13%           
  Complexity        ?      327           
=========================================
  Files             ?      152           
  Lines             ?     4055           
  Branches          ?      406           
=========================================
  Hits              ?     1749           
  Misses            ?     2184           
  Partials          ?      122

Impacted Files	Coverage Δ	Complexity Δ
...c/main/kotlin/kategory/instances/ConstInstances.kt	`68.42% <100%> (ø)`	`0 <0> (?)`
...main/kotlin/kategory/typeclasses/TraverseFilter.kt	`20% <20%> (ø)`	`0 <0> (?)`
...egory-core/src/main/kotlin/kategory/data/Option.kt	`55.76% <50%> (ø)`	`8 <1> (?)`
...tegory-core/src/main/kotlin/kategory/data/Const.kt	`53.33% <50%> (ø)`	`4 <1> (?)`
...e/src/main/kotlin/kategory/typeclasses/Composed.kt	`68.08% <54.54%> (ø)`	`0 <0> (?)`
...gory-core/src/main/kotlin/kategory/data/OptionT.kt	`41.81% <75%> (ø)`	`8 <2> (?)`
...main/kotlin/kategory/instances/OptionTInstances.kt	`86.95% <83.33%> (ø)`	`0 <0> (?)`
...rc/main/kotlin/kategory/laws/TraverseFilterLaws.kt	`9.09% <9.09%> (ø)`	`1 <1> (?)`

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 c5d26d6...dc7be24. Read the comment docs.

tonilopezmr · 2017-09-05T18:35:48Z

kategory-test/src/main/kotlin/kategory/UnitSpec.kt

@@ -20,6 +20,7 @@ abstract class UnitSpec : StringSpec() {
            Option.monad()
            Option.foldable()
            Option.traverse()
+            Option.traverseFilter()


It's magic!

raulraja

@tonilopezmr this is good to go but you have to update this branch with master and make sure all your declared instances provide both an implicit object and tests as they are now for all instances where we can prove they can be resolved implicitly.

pakoito · 2017-09-14T00:42:23Z

kategory-core/src/test/kotlin/kategory/data/OptionTTest.kt

    val EQ_ID: Eq<HK<OptionTKindPartial<IdHK>, Int>> = Eq { a, b ->
        a.value() == b.value()
    }

+    val EQ_OPTION = object : Eq<HK<OptionTKindPartial<OptionHK>, Int>> {


There's a new, easier constructor. Try Eq { a, b -> a.value() == b.value() } and let the type inference do the rest!

pakoito · 2017-09-14T00:43:42Z

kategory-core/src/test/kotlin/kategory/data/OptionTTest.kt

+                a.ev().value == b.ev().value
+    }
+
+    val EQ_NESTED_OPTION = object : Eq<HK<OptionTKindPartial<OptionHK>, HK<OptionTKindPartial<OptionHK>, Int>>> {


It may be that you could make the three Eq instances into a single method parametrized in T.

I have added

fun <T> EQ(): Eq<T> = Eq { a, b -> a.value() == b.value() }

but it doesn't work

kategory/kategory-core/src/test/kotlin/kategory/data/OptionTTest.kt: (12, 24): Unresolved reference. None of the following candidates is applicable because of receiver type mismatch: public fun <A, T> ConstKind<???, ???> /* = HK<HK<ConstHK, ???>, ???> */.value(): ??? defined in kategory public fun <A> IdKind<???> /* = HK<IdHK, ???> */.value(): ??? defined in kategory public inline fun <F, A> OptionTKind<???, ???> /* = HK<HK<OptionTHK, ???>, ???> */.value(): HK<???, Option<???>> defined in kategory public inline fun <F, W, A> WriterTKind<???, ???, ???> /* = HK<HK2<[ERROR : Unknown type parameter 3], [ERROR : Unknown type parameter 4], [ERROR : Unknown type parameter 5]> /* = HK<HK<WriterTHK, ???>, ???> */, ???> */.value(): HK<???, Tuple2<???, ???>> defined in kategory

I don't know why ヽ(。_°)ノ

It has to be Eq<HK<OptionTKindPartial>. Let's chat about why in Slack.

pakoito · 2017-09-15T19:52:41Z

kategory-test/src/main/kotlin/kategory/laws/TraverseFilterLaws.kt

+            )
+
+    inline fun <reified F> identityTraverseFilter(FT: TraverseFilter<F>, GA: Applicative<F> = applicative<F>(), crossinline cf: (Int) -> HK<F, Int>, EQ: Eq<HK<F, HK<F, Int>>> = Eq.any()) =
+            forAll(genConstructor(genIntSmall(), cf), { fa: HK<F, Int> ->


If you have an applicative of F you shouldn't need a constructor function: genApplicative() does the same job!

https://github.com/kategory/kategory/blob/master/kategory-test/src/main/kotlin/kategory/generators/Generators.kt#L5

pakoito

Nice PR! One small tweak to the tests and we're good to go!

Co-authored-by: Rachel M. Carmena <[email protected]>

tonilopezmr added 6 commits September 5, 2017 01:09

Add TraverseFilter

97a92cd

Add TraverseFilter in Const

b01db24

Added ComposedTraverseFilter

fffe5ec

Rename TraverserFilter.compose method to composeFilter

25b590f

Add traverseFilter in Option and OptionT

88d7309

Add identityTraverseFilter law to let raul check it

c4e3a7c

tonilopezmr added the work in progress label Sep 5, 2017

tonilopezmr self-assigned this Sep 5, 2017

tonilopezmr requested a review from raulraja September 5, 2017 13:58

raulraja mentioned this pull request Sep 5, 2017

Roadmap #69

Closed

54 tasks

tonilopezmr requested review from pakoito and JorgeCastilloPrz September 5, 2017 15:48

tonilopezmr added 6 commits September 5, 2017 17:49

Fix Identity law

13dddde

Fix detekt

ed5a2ba

Removed unnused traverse override

714c509

Added filterA consistent with traverseFilter law

f248388

TraverseFilterLaws include TraverseLaws

c1edbca

Merge branch 'toni-traversefilter' into toni-no-push

6a3e0d7

raulraja requested changes Sep 5, 2017

View reviewed changes

Removed FunctorLaws in TraverseFilterLaws

3866bba

Return the most concrete type in Const traverse and traverseFilter

c47dfa7

tonilopezmr commented Sep 5, 2017

View reviewed changes

tonilopezmr added 7 commits September 5, 2017 21:13

Format code

3176f88

Merge branch 'master' into toni-traversefilter

d87cf53

Merge branch 'toni-traversefilter' into toni-no-push

8776e8c

Fix filterA consistent with traverse filter thanks to @pakoito 👌 ❤️ 🔝

64f57c9

Merged branch 'master' into toni-traverseFilter

cfc14bd

Moved TraverseFilter to kategory-core

1396499

Merge branch 'master' into toni-traversefilter

2fa2218

raulraja requested changes Sep 13, 2017

View reviewed changes

tonilopezmr added 3 commits September 14, 2017 00:40

Fix conflicts

47c71ef

Fix tests

4b7863a

Resolve conflicts

84a6939

pakoito reviewed Sep 14, 2017

View reviewed changes

tonilopezmr added 3 commits September 14, 2017 19:03

Fix detekt?

0c28023

EQ in a single method

626a3fb

Merge branch 'master' into toni-traversefilter

a23d3e4

pakoito reviewed Sep 15, 2017

View reviewed changes

pakoito approved these changes Sep 15, 2017

View reviewed changes

pakoito and others added 2 commits September 15, 2017 23:32

Merge branch 'master' into toni-traversefilter

06f0966

Fix laws. Remove weird test in Const.

dc7be24

pakoito merged commit 13319e8 into master Sep 15, 2017

pakoito deleted the toni-traversefilter branch September 15, 2017 23:00

tonilopezmr removed the work in progress label Sep 16, 2017

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

Configuration: Add actions timeout (#255)

761ce00

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

Update README.md (#255)

43efb75

Co-authored-by: Rachel M. Carmena <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add TraverseFilter #255

Add TraverseFilter #255

tonilopezmr commented Sep 5, 2017 •

edited

Loading

raulraja Sep 5, 2017

tonilopezmr Sep 5, 2017 •

edited

Loading

raulraja Sep 5, 2017

raulraja Sep 5, 2017

raulraja Sep 5, 2017

tonilopezmr Sep 5, 2017

codecov-io commented Sep 5, 2017 •

edited

Loading

tonilopezmr Sep 5, 2017

raulraja left a comment

pakoito Sep 14, 2017 •

edited

Loading

pakoito Sep 14, 2017

tonilopezmr Sep 15, 2017

pakoito Sep 15, 2017

pakoito Sep 15, 2017 •

edited

Loading

pakoito left a comment

Add TraverseFilter #255

Add TraverseFilter #255

Conversation

tonilopezmr commented Sep 5, 2017 • edited Loading

Choose a reason for hiding this comment

tonilopezmr Sep 5, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Sep 5, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

raulraja left a comment

Choose a reason for hiding this comment

pakoito Sep 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pakoito Sep 15, 2017 • edited Loading

Choose a reason for hiding this comment

pakoito left a comment

Choose a reason for hiding this comment

tonilopezmr commented Sep 5, 2017 •

edited

Loading

tonilopezmr Sep 5, 2017 •

edited

Loading

codecov-io commented Sep 5, 2017 •

edited

Loading

pakoito Sep 14, 2017 •

edited

Loading

pakoito Sep 15, 2017 •

edited

Loading