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

Add EitherT and IorT constructors from Option with monadic default #3426

Conversation

ivan-klass
Copy link
Contributor

@ivan-klass ivan-klass commented May 24, 2020

In practice the OptionT[F, A] => EitherT[F, E, A] transformation may also require the left to be calculated as F[E]. I didn't find anything handy neither in OptionT nor EitherT having to work-around with .leftSemiflatMap(identity) like

OptionT(...) 
  .toRight(fe) // EitherT[F, F[E], A]
  .leftSemiflatMap(identity)

or

EitherT.fromOptionF(..., fe) // EitherT[F, F[E], A]
  .leftSemiflatMap(identity)

@codecov-commenter
Copy link

codecov-commenter commented May 24, 2020

Codecov Report

Merging #3426 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3426      +/-   ##
==========================================
+ Coverage   91.64%   91.65%   +0.01%     
==========================================
  Files         381      381              
  Lines        8268     8278      +10     
  Branches      225      225              
==========================================
+ Hits         7577     7587      +10     
  Misses        691      691              
Flag Coverage Δ
#scala_version_212 91.64% <100.00%> (+0.01%) ⬆️
#scala_version_213 91.44% <100.00%> (+0.01%) ⬆️
Impacted Files Coverage Δ
core/src/main/scala/cats/data/EitherT.scala 98.53% <100.00%> (+0.02%) ⬆️
core/src/main/scala/cats/data/IorT.scala 97.84% <100.00%> (+0.06%) ⬆️
core/src/main/scala/cats/data/OptionT.scala 96.15% <100.00%> (+0.06%) ⬆️

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 c067cac...74a6bc0. Read the comment docs.

@ivan-klass ivan-klass force-pushed the create-EitherT-from-option-and-monad-default branch from a04e4e1 to 8bbb5f1 Compare May 24, 2020 15:17
@ivan-klass ivan-klass force-pushed the create-EitherT-from-option-and-monad-default branch from 8bbb5f1 to 74a6bc0 Compare May 24, 2020 15:21
Copy link
Member

@LukaJCB LukaJCB left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

@kailuowang
Copy link
Contributor

kailuowang commented Jun 3, 2020

Option, like several other data types, already has a liftTo[F] that can "lift" it into any F[_] with an ApplicativeError[F], which is the case for both IorT and EitherT.
For lifting to the left side, I believe we had a PR adding a raiseTo[F] but somehow I couldn't find it anymore.
I am thinking that instead of adding n - 1 methods to n datatypes to support transformation between we can just add a liftTo[F] and a raiseTo[F] to n data types?
In this example, we can add those to OptionT and F[Option]

update: I found the raiseTo PR #3358

@LukaJCB LukaJCB merged commit ba44484 into typelevel:master Jun 3, 2020
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.

can you address my last comment?

@kailuowang
Copy link
Contributor

Hi, @ivan-klass , sorry about the confusion, but another maintainer merged this PR while I was drafting the feedback. We are reverting the merge, would you mind resubmit this PR? I can re-add my feedback there. Thanks!

@ivan-klass
Copy link
Contributor Author

Sure, will re-submit with addressed suggestion

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