Implement InvariantCoyoneda
#3987
Conversation
| /** | ||
| * The list of transformer functions composed into a single function, to be lifted into `F` by `run`. | ||
| */ | ||
| final def k0: Pivot => A = Function.chain(ks0.reverse)(_).asInstanceOf[A] | ||
|
|
||
| /** | ||
| * The composed transformer function, to be lifted into `F` by `run`. | ||
| */ | ||
| final def k1: A => Pivot = Function.chain(ks1)(_).asInstanceOf[Pivot] |
There was a problem hiding this comment.
Are there better names for these? Also, I see the comments are broken, will fix.
There was a problem hiding this comment.
I think this is fine, just to keep it consistent with the co/contravariant versions.
| /** | ||
| * `F[A]` converts to `InvariantCoyoneda[F,A]` for any `F` | ||
| */ | ||
| def lift[F[_], A](fa: F[A]): InvariantCoyoneda[F, A] = |
There was a problem hiding this comment.
Why not preserve the Aux type here but do so in other places?
There was a problem hiding this comment.
Actually I don't see the reason to ever preserve the Aux type. I can't see how anyone can use it. Isn't it better to use the type that hides the Pivot?
There was a problem hiding this comment.
I'm just following the style of the other Coyonedas, happy to make this change.
There was a problem hiding this comment.
I think it is worth it if we have AndThen designed explicitly for this purpose but it is probably binary incompatible on the existing ones since you will change List to AndThen.
There was a problem hiding this comment.
If we are going to retain the Aux in other places I think this should be Aux[F, A, A].
There was a problem hiding this comment.
Makes sense. I made this change to all the Coyonedas.
| /** | ||
| * The list of the `Pivot => *` halves of the transformer functions, to be composed and lifted into `F` by `run`. | ||
| */ | ||
| private[cats] val ks0: List[Any => Any] |
There was a problem hiding this comment.
Do we not want to use AndThen here so we know the function composition is safe (since we control it)?
There was a problem hiding this comment.
I think so too, I was just following the style of the other Coyonedas. Should I make the change across the board?
|
@johnynek I reworked all the Coyonedas to use Re |
|
I don't follow your comment "seems binary compatible" since you had to add mima exclusions. It seems binary incompatible to me, but I believe it is safe because: 1. these types are sealed 2. the changed values were |
Apologies, "safe" is what I meant, I'll be more precise next time. Or, binary-compatible from the perspective of users anyway :) |
|
This looks good, are we waiting on anything else? |
This came up in typelevel/vault#264, but @tpolecat's stack was about to overflow 😝