Skip to content

WIP: Sketch of most simple bifunctor extension#197

Closed
johnynek wants to merge 1 commit intotypelevel:masterfrom
johnynek:oscar/bifunctor
Closed

WIP: Sketch of most simple bifunctor extension#197
johnynek wants to merge 1 commit intotypelevel:masterfrom
johnynek:oscar/bifunctor

Conversation

@johnynek
Copy link

@johnynek johnynek commented May 6, 2018

a sketch of what #189 might look like in a source compatible way.

Missing steps:

  1. make all the existing typeclasses just aliases in the cats.effect.package for the ones with parameterized E fixed to Throwable
  2. copy IO over but parameterize on E.
  3. add cats.effect package alias of IO[A] = bifunctor.IO[Throwable, A]

If you want to work with no errors, you can fix E = Nothing and then you know that Either[Nothing, A] == A which we can give an implementation of:

def justRight[A](e: Either[Nothing, A]): A =
  e match {
    case Right(a) => a
    case Left(nothing) => nothing // is a subtype of a
  }

If you want to use Unit or String, everything goes through.

I guess all the typeclasses are InvariantFunctors in E, so we can give those too if people want to perhaps give some embedding from E <=> Throwable which I guess can be done for some Es.

@codecov-io
Copy link

Codecov Report

Merging #197 into master will decrease coverage by 0.5%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #197      +/-   ##
==========================================
- Coverage   89.69%   89.19%   -0.51%     
==========================================
  Files          50       53       +3     
  Lines        1087     1092       +5     
  Branches       77       77              
==========================================
- Hits          975      974       -1     
- Misses        112      118       +6

@johnynek
Copy link
Author

johnynek commented May 6, 2018

Note, Fiber and Bracket already seem okay, and would not need to change.

Also, we can give an implementation of all the typeclasses for IO[Throwable, A] IO[String, A] and IO[Nothing, A] and that might be the common cases people case about.

We would need to weaken the Sync laws. Perhaps we say that delay(throw e) and suspend(throw e) is undefined but "should" catch NonFatals when E = Throwable. Or we could require that the typeclass has some Throwable => E that can embed a throwable somehow in E in the case of E = Nothing, I guess it has to throw eto return Nothing, but that seems correct if you have promised that there are no errors (and clearly that has been invalidated). Maybedef errorFromNonFatal(e: Throwable): Egets added toSyncto enable a lawdelay(throw e) == raiseError(errorFromNonFatal(e))`?

@johnynek
Copy link
Author

johnynek commented May 6, 2018

Of course, this isn’t really forcing a bifunctor on anything unless we require F[_, _] which probably means E is not on the typeclasses but instead on the methods.

Then we get where we want with a

def attempt[E, A](f: F[E, A]): F[Nothing, Either[E, A]]

@rossabaker
Copy link
Member

rossabaker commented May 6, 2018

Thanks for getting something concrete out here so quickly!

  1. bifunctor.IO[Throwable, A] doesn't catch from map and flatMap. This is controversial behavior, but would change if IO were a simple alias.

  2. These "bifunctors" don't have cats.Bifunctor instances. If we're not forcing Bifunctor, and it would be a radical imposition on things like monix.eval.Task to do so, then I think we should get bifunctor out of the package name. I might like a Bi- prefix, but Biasync is funny looking. Naming is hard.

  3. Instead of Sync[F] merely aliasing bifunctor.Sync[F, Throwable], what if Sync[F] extended bifunctor.Sync[F, Throwable] to preserve the existing laws? I'd then like to legislate that map and flatMap catch, but now we've eliminated Sync[bifunctor.IO[Throwable, A]]. Dang.

  4. I can't see how you'd implement errorFromNonFatal for polymorphic E. Wouldn't this approach require every instance to be a specialization?

  5. How much of fs2 and Monix would be expressed with the binary type classes instead of the unary ones? 0% doesn't mean this is a bad idea, but a substantial percentage would really sell this.

  6. Nothing is tricky with respect to dead code warnings. Scalaz fixed this with Void.

* the purpose of this function is to suspend side effects
* in `F`.
*/
def delay[A](thunk: => A): F[A] = suspend(pure(thunk))
Copy link
Member

Choose a reason for hiding this comment

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

The problem with this is that it no longer catches any errors, so F.delay(throw e) can't safely deal with errors :/

Copy link
Member

Choose a reason for hiding this comment

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

Doh, of course I'm now realizing you've already talked about that 😂 sorry! Disregard

@LukaJCB
Copy link
Member

LukaJCB commented May 6, 2018

FWIW I have a WIP branch for unexceptional type classes for #176, that could help with this, I can push this later today :)

@alexandru
Copy link
Contributor

This is a good start, but I'd like to cleanup the issue tracker a bit, otherwise important issues can get lot in noise.

We'll probably get back to it, but let's close this for now.

@alexandru alexandru closed this Nov 9, 2018
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