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 a way to create a Kleisli with just F[B] #1090

Merged
merged 4 commits into from
Jun 8, 2016

Conversation

tbrown1979
Copy link

I'm new to Kleislis and found myself wanting a function like this. Scalaz has a .liftKleisli function, but I wasn't sure what would be preferred: Kleisli.pureF(fa) or fa.liftKleisli.

@codecov-io
Copy link

codecov-io commented Jun 4, 2016

Current coverage is 88.11%

Merging #1090 into master will decrease coverage by 0.03%

  1. 3 files (not in diff) in ...main/scala/cats/data were modified. more
  2. 1 files (not in diff) in .../src/main/scala/cats were modified. more
@@             master      #1090   diff @@
==========================================
  Files           224        224          
  Lines          2844       2845     +1   
  Methods        2787       2792     +5   
  Messages          0          0          
  Branches         52         48     -4   
==========================================
  Hits           2507       2507          
- Misses          337        338     +1   
  Partials          0          0          

Sunburst

Powered by Codecov. Last updated by 3608287...4a5fea2

@edmundnoble
Copy link
Contributor

Personally I would be in favour of naming this Kleisli.const, as I think that illustrates its meaning more effectively. pureF might not work so well, because pure implies a lack of effects (as in Applicative.pure) but pureF's argument would be an F[B], which may have effects. liftKleisli as a postfix operator sounds alright if it's that common a use-case, but you'd need to also add KleisliSyntax for a place to put it.

@tbrown1979
Copy link
Author

@edmundnoble Thanks for the response. I was looking for a more appropriate name and Kleisli.const seems much better. I'll make that change. I'll also look into adding the .liftKleisli syntax as some people might prefer that.

@non
Copy link
Contributor

non commented Jun 5, 2016

I second the Kleisli.const suggestion.

@ceedubs
Copy link
Contributor

ceedubs commented Jun 5, 2016

I think I'd be inclined to call this .liftT, and then the liftT implementation on the TransLift instance could simply delegate through to it.

@tbrown1979
Copy link
Author

Okay, so I've changed the name to .const but can certainly change it to .liftT if that is preferred. Either way, I have changed TransLift instance to use the Kleisli.const now.

@tbrown1979
Copy link
Author

I think I will forego the .liftKleisli for this pull request and add that in another, if we're okay with that.

@non
Copy link
Contributor

non commented Jun 7, 2016

👍 Thanks!

@ceedubs
Copy link
Contributor

ceedubs commented Jun 7, 2016

I think that everywhere else that we have an analogous method on other monad transformers it's called liftF or liftT, so I'd prefer to be consistent (or even just use lift). It may be an unfounded concern, but I don't want people to think that this is related to Const. I'll let another person weigh in -- I'm happy to have this merged if people align on const.

@peterneyens
Copy link
Collaborator

peterneyens commented Jun 7, 2016

Looking at similar methods on the other monad transformers there is no real consensus at the moment, probably because the methods on XorT and WriterT have to deal with their specific context.

Tranformer F[A] => MT[F, ... , A]
OptionT OptionT.liftF
XorT XorT.right / Xort.left
WriterT WriterT.putT / WriterT.valueT
StateT none
Kleisli none

Even though Free is not a monad transformer it also uses Free.liftF to go from F[A] to Free[F, A].

I like Kleisli.const in this case, but concur with @ceedubs that it is important to be consistent. Since we have the type class TransLift with liftT, I think that it's a good idea to use MT.lift or MT.liftF if there is no better specific name (like for XorT and WriterT).

OptionT.liftF(List(1,2))
List(1,2).liftT[OptionT]

Kleisli.lift[List, Int, Int](List(1,2)) // ?
List(1,2).liftT[({type λ[α[_], β] = Kleisli[α, Int, β]})#λ] // -> liftKleisli[Int] ?

@ceedubs
Copy link
Contributor

ceedubs commented Jun 7, 2016

@peterneyens thanks for the helpful writeup. It looks like I definitely didn't have the facts on my side when I claimed that this was liftT or liftF everywhere else.

@tbrown1979
Copy link
Author

I've renamed from .const to .lift to remain a little more consistent with naming. 😄

@ceedubs
Copy link
Contributor

ceedubs commented Jun 8, 2016

👍 thanks!

@ceedubs
Copy link
Contributor

ceedubs commented Jun 8, 2016

@non your 👍 was for const -- are you okay with lift?

@non
Copy link
Contributor

non commented Jun 8, 2016

👍

@non non merged commit a315749 into typelevel:master Jun 8, 2016
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.

6 participants