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

Adding more contravariant instances #929

Merged
merged 1 commit into from
Jun 14, 2016

Conversation

DavidGregory084
Copy link
Member

Some more work on #579.

implicit val orderInstances: cats.functor.Contravariant[Order] =
new cats.functor.Contravariant[Order] {
def contramap[A, B](fa: Order[A])(f: B => A): Order[B] = fa.on(f)
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not 100% sure where instances for types from Algebra belong - happy to move them somewhere a bit less ad-hoc

@codecov-io
Copy link

codecov-io commented Mar 11, 2016

Current coverage is 89.84%

Merging #929 into master will increase coverage by 0.95%

  1. 6 files (not in diff) in ...main/scala/cats/free were created. more
  2. 1 files (not in diff) in .../src/main/scala/cats were created. more
  3. 2 files (not in diff) in ...cats/laws/discipline were modified. more
    • Hits -5
  4. 5 files (not in diff) in ...main/scala/cats/laws were modified. more
    • Hits -2
  5. 7 files (not in diff) in ...in/scala/cats/syntax were modified. more
    • Misses +13
    • Hits -18
  6. 9 files (not in diff) in .../main/scala/cats/std were modified. more
    • Misses +15
    • Hits +35
  7. 3 files (not in diff) in ...n/scala/cats/functor were modified. more
    • Misses +10
    • Hits -11
  8. 8 files (not in diff) in ...main/scala/cats/data were modified. more
    • Misses -2
    • Hits -40
  9. 14 files (not in diff) in .../src/main/scala/cats were modified. more
    • Misses +27
    • Hits -19
  10. 2 files in ...cats/laws/discipline were modified. more
    • Hits +3
@@             master    #929   diff @@
=======================================
  Files           226     180     -46   
  Lines          2916    2177    -739   
  Methods        2865       0   -2865   
  Branches         48      42      -6   
=======================================
- Hits           2592    1956    -636   
+ Misses          324     221    -103   
  Partials          0       0           

Sunburst

Powered by Codecov. Last updated by 00839fb...ad4ab8b

@DavidGregory084
Copy link
Member Author

I was able (I think) to write an implementation for StateT as:

def contramap[B](f: B => A)(implicit F: Contravariant[F], M: Monad[F]) =
  StateT(s1 =>
    F.contramap(run(s1)) {
      case (s2,  a) => (s2,  f(a)) 
    }
  ) 

However, this would typically mean resolving implicits for e.g Contravariant[Cokleisli[Option, ?, Int]] and Monad[Cokleisli[Option, Int, ?]] at the same time - I'm not really sure how to arrange that.

@DavidGregory084 DavidGregory084 force-pushed the contravariant-instances branch from 2924820 to ab31ab1 Compare March 28, 2016 15:56
@non
Copy link
Contributor

non commented May 30, 2016

Hi @DavidGregory084, sorry for the long wait!

These instances look great! Would you be willing to update this PR?

As far as instances for the kernel (e.g. contravariant functors for Eq, PartialOrder, and Order) I would actually recommend putting those in the Contravariant companion object (in core). I would also recommend adding a note that the resulting instances are only law-abiding if the functions provided are one-to-one (injections).

@DavidGregory084 DavidGregory084 force-pushed the contravariant-instances branch from ab31ab1 to 04b78ac Compare May 31, 2016 21:34
@DavidGregory084
Copy link
Member Author

No problem, this should be up to date now 😸

@ceedubs
Copy link
Contributor

ceedubs commented Jun 5, 2016

Sorry - we've been merging some sweeping PRs recently that have caused a lot of merge conflicts :(. Would you mind updating again?

@DavidGregory084 DavidGregory084 force-pushed the contravariant-instances branch from 04b78ac to 39fbd28 Compare June 5, 2016 21:56
@DavidGregory084
Copy link
Member Author

@ceedubs Yeah, that's fine, should be up to date again :)

@ceedubs
Copy link
Contributor

ceedubs commented Jun 6, 2016

👍 thanks a bunch, @DavidGregory084! @non would you be willing to do a review before this goes stale again?

@non
Copy link
Contributor

non commented Jun 14, 2016

Looks great, thanks! 👍

@non non merged commit de5f538 into typelevel:master Jun 14, 2016
@DavidGregory084 DavidGregory084 deleted the contravariant-instances branch August 12, 2016 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants