Add FunctorFilter #224

JorgeCastilloPrz · 2017-08-22T15:21:14Z

Fixes: #223

Added FunctorFilter
Added FunctorFilterLaws
Added ComposedFunctorFilter and OptionTFunctor FunctorFilter instances.
Tested laws using OptionTFunctor.

codecov-io · 2017-08-22T15:29:14Z

Codecov Report

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

@@            Coverage Diff            @@
##             master     #224   +/-   ##
=========================================
  Coverage          ?   49.84%           
  Complexity        ?      207           
=========================================
  Files             ?      103           
  Lines             ?     2329           
  Branches          ?      279           
=========================================
  Hits              ?     1161           
  Misses            ?     1056           
  Partials          ?      112

Impacted Files	Coverage Δ	Complexity Δ
...tegory/src/main/kotlin/kategory/arrow/Function1.kt	`62.5% <ø> (ø)`	`1 <0> (?)`
kategory/src/main/kotlin/kategory/data/Composed.kt	`29.09% <0%> (ø)`	`0 <0> (?)`
.../main/kotlin/kategory/typeclasses/FunctorFilter.kt	`0% <0%> (ø)`	`0 <0> (?)`
.../src/main/kotlin/kategory/arrow/PartialFunction.kt	`29.41% <0%> (ø)`	`1 <0> (?)`
.../src/main/kotlin/kategory/generators/Generators.kt	`74.07% <100%> (ø)`	`0 <0> (?)`
...main/kotlin/kategory/instances/OptionTInstances.kt	`94.44% <100%> (ø)`	`0 <0> (?)`
kategory/src/main/kotlin/kategory/data/OptionT.kt	`21.27% <33.33%> (ø)`	`4 <0> (?)`
...src/main/kotlin/kategory/laws/FunctorFilterLaws.kt	`5.55% <5.55%> (ø)`	`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 1a7dd13...db4614e. Read the comment docs.

pakoito · 2017-08-22T19:44:26Z

kategory-test/src/main/kotlin/kategory/generators/Generators.kt

@@ -55,3 +55,8 @@ fun genIntPredicate(): Gen<(Int) -> Boolean> =
                    { it % absNum == absNum - 1 })
            )
        }
+
+fun <B> genOption(genB: Gen<B>): Gen<Option<B>> =


This method could be implemented using genApplicative(). In any case, to properly test Option we'd need to generate at all possible cases. What I'd suggest is to randomise None to a low percent by capturing a Gen.int() that'll run on every generate and and checking modulus 20 for 5% chance.

👍 fixed. I made it randomized to generate None and Some on a 5% - 95% basis.

You need to capture genIntSmall() as a field inside the object and call generate() on it inside generate(), else you won't get adequate distributions as it restarts the seed every time.

pakoito · 2017-08-22T20:38:09Z

kategory/src/main/kotlin/kategory/arrow/Function1.kt

@@ -4,7 +4,7 @@ fun <I, O> ((I) -> O).k(): Function1<I, O> = Function1(this)

 operator fun <I, O> Function1Kind<I, O>.invoke(i: I): O = this.ev().f(i)

-@higherkind class Function1<I, out O>(val f: (I) -> O) : Function1Kind<I, O> {
+@higherkind open class Function1<I, out O>(val f: (I) -> O) : Function1Kind<I, O> {


Who inherits from Function1?

Nobody. It was being inherited by Lifted in the beginning, but no need for it anymore.

pakoito · 2017-08-22T20:39:38Z

kategory/src/main/kotlin/kategory/data/OptionT.kt

@@ -45,6 +45,13 @@ package kategory
        inline fun <reified F> monoidK(MF: Monad<F> = kategory.monad<F>()): MonoidK<OptionTKindPartial<F>> = object : OptionTMonoidK<F> {
            override fun F(): Monad<F> = MF
        }
+
+        inline fun <reified F> functorFilter(MF: Monad<F> = kategory.monad<F>(), FF: Functor<F> = kategory.functor()): FunctorFilter<OptionTKindPartial<F>> =


Monad implies functor. You only need 1 parameter.

pakoito · 2017-08-22T20:40:25Z

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

+
+    fun FF(): Functor<F>
+
+    fun MF(): Monad<F>


Monad implies functor. One should be enough.

pakoito · 2017-08-22T20:42:52Z

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

+
+object FunctorFilterLaws {
+
+    inline fun <reified F> laws(AP: Applicative<F> = applicative(), FFF: FunctorFilter<F> = functorFilter(), EQ: Eq<HK<F, Int>>): List<Law> =


Some FunctorFilters may not be Applicatives. I'd suggest passing a constructor function, using genConstructor() and inlining AP::pure in the current callsites instead.

What do you mean ? FunctorFilterLaws depend on FunctorLaws which need an Applicative to run, that's why it's being passed here, but the FunctorFilterLaws are being tested using the FunctorFilter passed as a second argument.

FunctorLaws has 2 constructor, one convenient one with Applicative in the case that you're testing anything that inherits from applicative, and another that takes a constructor function for other cases. In this case as we're testing something that doesn't inherit from applicative please use the second one.

pakoito · 2017-08-22T22:21:39Z

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

+        testLaws(FunctorFilterLaws.laws(
+                OptionT.applicative(Option),
+                OptionT.functorFilter(),
+                Eq.any()))


I believe it may need an instance of Eq<OptionTPartial<Option, Int>>, as we've seen similar issues with other tests before.

… and there

…lterLaws

…l the possible cases

JorgeCastilloPrz · 2017-08-23T11:44:40Z

Back to you @pakoito

pakoito · 2017-08-23T12:27:01Z

kategory/src/main/kotlin/kategory/data/Composed.kt

+    companion object {
+        inline operator fun <reified F, reified G> invoke(FF: Functor<F> = functor(), FFG: FunctorFilter<G> = functorFilter()): ComposedFunctorFilter<F, G> =
+                object : ComposedFunctorFilter<F, G> {
+                    override fun F(): Functor<F> = FF


FunctorFilter implies Functor, you don't need the second parameter.

Well that's inherited from ComposedFunctor, following cats implementation.

I mean you can assign FFG to this field too.

Derp, I see.

pakoito · 2017-08-23T12:28:04Z

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

+                OptionT.functorFilter(),
+                object : Eq<HK<OptionTKindPartial<OptionHK>, Int>>{
+                    override fun eqv(a: HK<OptionTKindPartial<OptionHK>, Int>, b: HK<OptionTKindPartial<OptionHK>, Int>): Boolean =
+                            a.ev().value == b.ev().value


Are we sure we can compare Option with ==, or do we need an Eq instance?

We are doing the same for other laws being tested on this class. I've changed it now to use the already created OptionTFIdEq instance (on top part of this test class).

…cative

JorgeCastilloPrz · 2017-08-23T13:48:41Z

Back to you again @pakoito

pakoito

Boom! Closer to MTL now!

…#224)

JorgeCastilloPrz requested review from raulraja and pakoito August 22, 2017 15:21

JorgeCastilloPrz and others added 9 commits August 22, 2017 17:21

push to let raul check something

401d51c

remove wildcard import

2076039

alternative non optimized impl

ddc4a40

alternative non optimized impl

a641cfc

Replace Lifted#invoke impl to be in terms of applyOrElse

c0eaf1f

complete FunctorFilter

e1e2a06

write laws

3215da6

add OptionT.functorFilter instance and use it to test laws

6ec7521

add ComposedFunctorFilter

ec6cfed

JorgeCastilloPrz force-pushed the jorge-functorfilter branch from f154b6c to ec6cfed Compare August 22, 2017 15:21

raulraja mentioned this pull request Aug 22, 2017

Roadmap #69

Closed

54 tasks

pakoito reviewed Aug 22, 2017

View reviewed changes

JorgeCastilloPrz added 3 commits August 23, 2017 12:06

remove open from Function1 and not needed Functor arguments from here…

1633fd8

… and there

pass Eq<HK<OptionTKindPartial<OptionHK>, Int>> instance for FunctorFi…

1b77bca

…lterLaws

randomize Option generator to make it a complete function covering al…

0fd068c

…l the possible cases

pakoito reviewed Aug 23, 2017

View reviewed changes

JorgeCastilloPrz added 2 commits August 23, 2017 15:27

capture smallInt on a field inside the generator object

77263e1

switch FunctorFilterLaws to use constructor function instead of appli…

db4614e

…cative

pakoito approved these changes Aug 23, 2017

View reviewed changes

JorgeCastilloPrz merged commit 4053118 into master Aug 23, 2017

JorgeCastilloPrz deleted the jorge-functorfilter branch August 23, 2017 14:09

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

Configuration: remove transitive dependency and update configurations (…

bc6cbc7

…#224)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add FunctorFilter #224

Add FunctorFilter #224

JorgeCastilloPrz commented Aug 22, 2017

codecov-io commented Aug 22, 2017 •

edited

Loading

pakoito Aug 22, 2017 •

edited

Loading

JorgeCastilloPrz Aug 23, 2017

pakoito Aug 23, 2017 •

edited

Loading

pakoito Aug 22, 2017

JorgeCastilloPrz Aug 23, 2017

pakoito Aug 22, 2017

pakoito Aug 22, 2017

pakoito Aug 22, 2017 •

edited

Loading

JorgeCastilloPrz Aug 23, 2017

pakoito Aug 23, 2017 •

edited

Loading

JorgeCastilloPrz Aug 23, 2017

pakoito Aug 22, 2017

JorgeCastilloPrz Aug 23, 2017

JorgeCastilloPrz commented Aug 23, 2017

pakoito Aug 23, 2017

JorgeCastilloPrz Aug 23, 2017

pakoito Aug 23, 2017

pakoito Aug 23, 2017

pakoito Aug 23, 2017

JorgeCastilloPrz Aug 23, 2017

JorgeCastilloPrz commented Aug 23, 2017

pakoito left a comment


		object FunctorFilterLaws {

		inline fun <reified F> laws(AP: Applicative<F> = applicative(), FFF: FunctorFilter<F> = functorFilter(), EQ: Eq<HK<F, Int>>): List<Law> =

Add FunctorFilter #224

Add FunctorFilter #224

Conversation

JorgeCastilloPrz commented Aug 22, 2017

codecov-io commented Aug 22, 2017 • edited Loading

Codecov Report

pakoito Aug 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pakoito Aug 23, 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

pakoito Aug 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pakoito Aug 23, 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

JorgeCastilloPrz commented Aug 23, 2017

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

Choose a reason for hiding this comment

JorgeCastilloPrz commented Aug 23, 2017

pakoito left a comment

Choose a reason for hiding this comment

codecov-io commented Aug 22, 2017 •

edited

Loading

pakoito Aug 22, 2017 •

edited

Loading

pakoito Aug 23, 2017 •

edited

Loading

pakoito Aug 22, 2017 •

edited

Loading

pakoito Aug 23, 2017 •

edited

Loading