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

MTL removal plan #1616

Closed
edmundnoble opened this issue Apr 18, 2017 · 19 comments
Closed

MTL removal plan #1616

edmundnoble opened this issue Apr 18, 2017 · 19 comments
Milestone

Comments

@edmundnoble
Copy link
Contributor

edmundnoble commented Apr 18, 2017

So, decision time for #1603. Here is my proposal for 1.0.0:

a) Create a library, Transmogrifier, which will contain monad transformer classes, using another law-testing methodology and type class encoding separate from cats'.
a) Remove MonadReader, MonadWriter, MonadState, FunctorFilter, TraverseFilter, MonadFilter and MonadCombine from cats, to be substituted in Transmogrifier.
b) Provide operations on Alternative (or Monad; if there's a preference let me know) which can stand in for the MonadCombine operations: unite and separate.
c) Keep MonadError in cats, at least for now. Edit: WIth an analogue in Transmogrifier.

@adelbertc
Copy link
Contributor

Will there be a MonadError analogue in this lib? Also can we just call it cats-mtl or something? :P

@edmundnoble
Copy link
Contributor Author

Yes and yes. Shall I make an issue later to vote on the name once this is accepted?

@kailuowang
Copy link
Contributor

Yes to #MTLexit

@kailuowang
Copy link
Contributor

paging @typelevel/cats

@djspiewak
Copy link
Member

I'm assuming that the MTL encoding will be something like the following?

trait WithState[F[_], S] {
  def set(s: S): F[Unit]
  def get: F[S]
  def modify(f: S => S): F[Unit]
}

So no direct compositional encoding (a la scato), and just expecting users to bring in Monad on their own?

@edmundnoble
Copy link
Contributor Author

Exactly. And a compositional encoding is used for "bundling" type classes on the user end.

@alexandru
Copy link
Member

I vote for cats-mtl as the name.

When picking names, please think of non-native English speakers. I don't know what "mogrify" or "trans-mogrification" means and I'd probably get spelling wrong when searching for it :-)

@ceedubs
Copy link
Contributor

ceedubs commented Apr 24, 2017

I'm 👍 on this assuming that the items in this proposal are an ordered list :). That is, I'd like to see this library take shape and show itself to work before we start removing things from cats.

I'm slightly hesitant to see FunctorFilter drop from cats-core, because I like the idea of providing a type class-based abstraction for filter and collect in the "core" of Cats, but I understand that these are messy/problematic in the current method of encoding type classes, so I'm happy to see how it plays out. Would this really be considered an MTL type class though? It seems like we are drawing the line more at type classes where subtyping becomes problematic and less at which type classes are related to monad transformers (TBH I wonder if MTL is even a bit of a misnomer for the haskell mtl library).

@kailuowang
Copy link
Contributor

@ceedubs how would you define "take shape and show itself to work"? how does that fit into cats' 1.0.0 roadmap?

@djspiewak
Copy link
Member

@alexandru Regarding the name, I'm pretty sure it's a reference to Calvin and Hobbes. You're absolutely right that it's a terrible name for non-native English speakers. It's actually pretty hard to remember how to spell it even as a native speaker. :-)

@edmundnoble
Copy link
Contributor Author

edmundnoble commented Apr 24, 2017

@ceedubs I'm in agreement on that point. I'm working on it at the moment; my hope is that the benefits are obvious from the user's perspective.

Edit: I believe there's a rigorous sense in which you can consider FunctorFilter a MTL class, because it's a RelativeMonad over Option.

@alexandru Agreed, I made the initial repo named cats-mtl.

@ceedubs
Copy link
Contributor

ceedubs commented Apr 25, 2017

@kailuowang by "take shape and show itself to work" I'd look for the following items:

  • The repo exists (easy step one :))
  • It has support for the types that we want to remove from Cats
  • It has some unit tests showing that it works in some of the currently problematic scenarios
  • It has some unit tests showing that it has a usable way to test laws for these MTL classes (@edmundnoble mentioned that these would be done in terms of Free I believe).

As far as how it fits into the Cats 1.0 roadmap, I'm hoping that these items could be accomplished first and that the relevant pieces of Cats could be gone by 1.0. But I'm not the one leading the charge on cats-mtl. Does that sound reasonable, @edmundnoble?

@wedens
Copy link
Contributor

wedens commented Apr 25, 2017

this article can be relevant to testing mtl-like typeclasses in terms of Free

@kailuowang
Copy link
Contributor

sounds like we have a consensus here. Should I move this ticket to "in dev" in (github project)? @edmundnoble ?

@kailuowang kailuowang added this to the 1.0.0-MF milestone Apr 25, 2017
@edmundnoble
Copy link
Contributor Author

edmundnoble commented Apr 25, 2017

Sounds good @kailuowang; got a basic repo (mostly just a clone of cats) up at edmundnoble/cats-mtl.

@wedens @ceedubs I'm leaning against using Free for testing, because that seems to mean a) user code needs to use Free and b) Free needs to leak into the definitions of transformers, meaning my goal of removing (Monad) constraints is dead out of the water. The law-testing indicated in that article appears to be RelativeMonad-based albeit based on an initial encoding; I'm going to try to final-tagless it up to move the constraint to the user.

@edmundnoble
Copy link
Contributor Author

Just a note: I am not removing the actual MTL transformers themselves from cats because we've got State implemented in terms of StateT, and Writer in terms of WriterT.

@edmundnoble
Copy link
Contributor Author

After asking in typelevel/cats, I've decided to remove ApplicativeError (with replacement in cats-mtl) as well.

@kailuowang
Copy link
Contributor

@edmundnoble you will move the methods to MonadError right?

@edmundnoble
Copy link
Contributor Author

✨ in 1.0.0-MF.

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

No branches or pull requests

7 participants