Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

contravariant coyoneda #2150

Merged
merged 3 commits into from
Feb 28, 2018
Merged

Conversation

tpolecat
Copy link
Member

This is mostly a copy/paste job from Coyoneda with the arrows flipped.

@codecov-io
Copy link

codecov-io commented Jan 12, 2018

Codecov Report

Merging #2150 into master will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2150      +/-   ##
==========================================
+ Coverage   94.67%   94.71%   +0.03%     
==========================================
  Files         328      329       +1     
  Lines        5536     5558      +22     
  Branches      199      214      +15     
==========================================
+ Hits         5241     5264      +23     
+ Misses        295      294       -1
Impacted Files Coverage Δ
...c/main/scala/cats/free/ContravariantCoyoneda.scala 100% <100%> (ø)
core/src/main/scala/cats/data/Ior.scala 98.48% <0%> (-1.52%) ⬇️
core/src/main/scala/cats/syntax/either.scala 99.15% <0%> (+0.01%) ⬆️
core/src/main/scala/cats/instances/function.scala 100% <0%> (+6.25%) ⬆️
core/src/main/scala/cats/UnorderedTraverse.scala 100% <0%> (+50%) ⬆️

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 5f30a73...f6c7f79. Read the comment docs.

}

/** `ContravariantCoyoneda[F, ?]` provides a conntravariant functor for any `F`. */
implicit def catsFreeCovariantFunctorForCovariantCoyoneda[F[_]]: Contravariant[ContravariantCoyoneda[F, ?]] =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be Contravariant here

final def run(implicit F: Contravariant[F]): F[A] = F.contramap(fi)(k)

/** Converts to `G[A]` given that `G` is a contravariant functor */
final def foldMap[G[_]](trans: F ~> G)(implicit G: Contravariant[G]): G[A] =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a test for this one as well?

@kailuowang
Copy link
Contributor

@tpolecat would you like to bring this one into the upcoming 1.1 release?

@tpolecat
Copy link
Member Author

Yeah I'll try to do that test this morning.

@kailuowang kailuowang added this to the 1.1 milestone Jan 31, 2018
@tpolecat
Copy link
Member Author

I added a kind of lame test that exercises foldMap … I couldn't come up with a contravariant functor that was anything other than some wrapping of ? => A. Happy to take suggestions but I think this may just be the nature of contravariant functors.

Copy link
Contributor

@kailuowang kailuowang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM, although I am not very familiar with the subject. It would be great if we can get 2 more reviews.

@kailuowang
Copy link
Contributor

@iravid any other feedback on this one?

@iravid
Copy link
Contributor

iravid commented Feb 7, 2018 via email

@LukaJCB
Copy link
Member

LukaJCB commented Feb 7, 2018

I'm very similar to Kai, the code looks very much okay to me, but I don't quite grasp the use cases, so if we could get one other review, maybe that'd be best.

@tpolecat
Copy link
Member Author

tpolecat commented Feb 7, 2018

As a practical matter the main use case is factoring out the boilerplate of writing a [stack-safe] contravariant functor. Simple example here.

@kailuowang kailuowang merged commit 97c08d5 into typelevel:master Feb 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants