Many small changes and cleanup. #41
Conversation
- a bunch of stuff pulled from quasar.contrib.matryoksha - renamed `FunctorT#map` and `TraverseT#traverse` to `mapT` and `traverseT`, both to match `embedT`/`projectT` and to avoid conflicting with Functor/Traverse instances on those types. - switched to the Typelevel Scala fork and upgraded kind-projector - cleaned up some constraints, etc. (eg, switching from `lift` to `∘`) - added some docs
|
I think increasing coverage (while writing docs) has to happen soon … |
Current coverage is 73.49% (diff: 51.76%)@@ master #41 diff @@
==========================================
Files 38 38
Lines 625 649 +24
Methods 613 634 +21
Messages 0 0
Branches 12 15 +3
==========================================
+ Hits 467 477 +10
- Misses 158 172 +14
Partials 0 0
|
| } | ||
| } | ||
| // TODO: Needs a better name. Something that evokes `A => B => (A, B)`, | ||
| // maybe like `tuple` or `uncurry`? |
There was a problem hiding this comment.
I like tuple. I think uncurry would look more like A => B => C => (A, B) => C?
| _.fold[List[A]](Nil) { case (a, (x, y)) => x ++ (a :: y) } | ||
|
|
||
| // NB: not in-place | ||
| def quicksort[A: Order]: List[A] => List[A] = { |
There was a problem hiding this comment.
I didn't see any tests for quicksort, join, and partition. If we're supporting them in the library it would be good to test them, maybe with scalacheck?
| object CoEnv extends CoEnvInstances { | ||
| def coEnv[E, F[_], A](v: E \/ F[A]): CoEnv[E, F, A] = CoEnv(v) | ||
|
|
||
| def hmap[F[_], G[_], A](f: F ~> G) = |
There was a problem hiding this comment.
What does the h stand for in these operations?
There was a problem hiding this comment.
“higher-kinded”, as in https://github.com/pa-ba/compdata/blob/master/src/Data/Comp/Multi/HFunctor.hs#L89 (these are just moved from ones we wrote in Quasar).
More of these type classes exist in #28, but the Cats naming scheme might lead us to FunctorK, etc. instead. Finally, there is work on adding polykinds to scalac, which would mean that FunctorK and Functor can be unified into a single type class, and then this operation becomes just map … which is actually a bit of a problem, because we have both HFunctor and Functor on this type currently …
| def takeUpTo[N, T, A](implicit N: Recursive.Aux[N, Option], T: Recursive.Aux[T, ListF[A, ?]]): Coalgebra[ListF[A, ?], (N, T)] = | ||
| pair => pair._1.project.fold[ListF[A, (N, T)]](NilF())(p => pair._2.project.map((p, _))) | ||
|
|
||
| def find[A](cond: A => Boolean): Algebra[ListF[A, ?], Option[A]] = { |
There was a problem hiding this comment.
I think we could generalize this away from ListF, to a more basic notion of find for some recursive structure. Not for this PR, but I think that sort of function, might be a way to help users more easily find the correct morphism.
I wonder if that should be a second library? Or at least a different package here. Basically, base matryoshka contains the impls of the morphisms, and then the usages package wraps the direct calls to the morphisms into functions with more commonly used signatures.
There was a problem hiding this comment.
There are common patterns we see in the connectors for example, with interpretM and freeCata.
A good approach could be to ask the question: "What questions do people answer with matryoshka?" For example: "I'd like to find all instances of a certain pattern and accumulate those (mapped to another type perhaps) into a list. How do I do that?" And then we wrap that all up in a function that calls para that takes as input the pattern to match on.
There was a problem hiding this comment.
Yeah, there is already a package for that – matryoshka.instances.fixedpoint.
Also, ListF is pretty general – it simply captures the notion of nested pairs that may terminate. It’s used for List/IList, as well as some stream-like structures, Seq, … maybe we could generalize to anything that’s Bifoldable. Worth exploring.
There was a problem hiding this comment.
I’m generally pretty wary about wrapping things up with their folds. I do it in matryoshka.instances.fixedpoint to illustrate how we can implement existing interfaces, but one of the big benefits of recursion schemes is the compositionality, and hiding the algebras (as we used to do in Quasar, but have moved away from) removes that. That’s why (for the most part) the algebras used by matryoshka.instances.fixedpoint are defined in matryoshka.patterns – you generally want to try to work with patterns instead of instances.
There was a problem hiding this comment.
Oh, and BTW, as of a couple PRs ago, freeCata is dead – it’s now just cata.
There was a problem hiding this comment.
@sellout I think that I disagree with that (the wrapping up of folds) to a degree, but don't have the time now to work it all out. So since it's well beyond the scope of this PR, we can come back to it later :)
|
Ok, updated this. Adding a test for quicksort was helpful, because it revealed an awkwardness in defining it as a function rather than a method (would have had to call |
|
This could use another review – want to try to update Quasar after this gets merged (just got frustrated by not being able to use |
|
LGTM - merging! |
FunctorT#mapandTraverseT#traversetomapTandtraverseT, both to matchembedT/projectTand to avoidconflicting with Functor/Traverse instances on those types.
liftto∘)