haskell / mtl

The Monad Transformer Library
http://www.haskell.org/haskellwiki/Monad_Transformers
Other
360 stars 63 forks source link

Added `liftMaybe` for lifting Maybe into MonadError #137

Open emlautarom1 opened 1 year ago

emlautarom1 commented 1 year ago

On #37 it was discussed the existence of liftMaybe:

    I think `lift` is fine, but would also like `liftMaybe`.

Originally posted by @ocharles in https://github.com/haskell/mtl/issues/37#issuecomment-325640686

This PR adds liftMaybe, also called note in some custom preludes like Universum and Protolude

On Universum, they suggest using maybeToRight, which involves liftMaybe e = liftEither . maybeToRight e. Personally, I'm not a big fan of such boilerplate.


Lifts a Maybe into any MonadError e, using a supplied e as the error if the Maybe is Nothing.

do { val <- liftMaybe e =<< action1; action2 }

where action1 returns a Maybe.

ocharles commented 1 year ago

While I did say liftMaybe 5 years ago (!) I no longer think this is the right name for it, but it's a useful operation to have. I know it as note (from https://hackage.haskell.org/package/errors-2.3.0/docs/Control-Error-Util.html#v:note)

emlautarom1 commented 1 year ago

While I did say liftMaybe 5 years ago (!) I no longer think this is the right name for it, but it's a useful operation to have. I know it as note (from https://hackage.haskell.org/package/errors-2.3.0/docs/Control-Error-Util.html#v:note)

Note (no pun intended) that note is a -> Maybe b -> Either a b. This is similar to other popular functions like maybeToRight or maybeToEither. This means that to get the same behavior as the proposed liftMaybe e, you need to liftEither . maybeToRight e, which I think is a bit annoying to write if you happen to have multiple Maybes. Compare:

example = do
  a <- liftEither $ maybeToRight "Error 1" $ ...
  b <- liftEither $ maybeToRight "Error 2" $ ...
  c <- liftEither $ maybeToRight "Error 3" $ ...
  return ()

with

example = do
  a <- liftMaybe "Error 1" $ ...
  b <- liftMaybe "Error 2" $ ...
  c <- liftMaybe "Error 3" $ ...
  return ()
ocharles commented 1 year ago

I'm not saying use note exactly as it is, just that turning a Maybe x into something that contains either an x or an error e is what I would call "noting" rather than lifting. I actually think errors would be better if it didn't choose Either/ExceptT but instead used MonadError. This would mean:

note :: MonadError e m => e -> Maybe a -> m a
emilypi commented 1 year ago

I'm actually not opposed to this, since I often find myself swallowing useless error messages and committing wholesale to algebraic blindness (vis. boolean blindness) in a bunch of important use-cases. This is one of those things that we reinvent over and over and would be good to include in mtl. Go for it!

kozross commented 1 year ago

Also in favour. Would be nice to @since this: minor bump only.

emilypi commented 1 year ago

@kozross I'd be in favor of waiting until the next major version bump due to the fact that liftMaybe is defined in many places (including Agda): https://hoogle.haskell.org/?hoogle=liftMaybe

emlautarom1 commented 1 year ago

There are multiple liftMaybes in the wild:

note has only two common interpretations:

As discussed, naming might not be the best and something like note would be better since it closely resembles the current state of the ecosystem, but I think that following the convention lift<type> is easier for discovery: if I'm already handling an Either inside a MonadError with liftEither I would expect something like liftMaybe to do the same with a Maybe.

Lev135 commented 1 year ago

For me it seems like note is better, than liftMaybe. Note, that currently available liftMaybes produce an empty error (()/empty/mzero). And that's I'd expect from lifting operation: Maybe is lifted without adding anything. Compare with liftEither: Either already has e and we also lift it without changing anything. lift from MonadTrans has similar behavior.

lift is easier for discovery

I think that's very weak argument: most of the users will search this function by it's signature and the naming doesn't matter in this stage. Looking at the code, I think, it should be obvious enough what this function does, even if you haven't seen it before.

emilypi commented 1 year ago

If it's obvious what the function does, and users search by type, then the same argument applies to changing the name of the thing to note: I don't support breaking from the conventions of this library just to support an extremely weak "intuition", which does not exist for non-fluent English speakers. liftMaybe makes sense in the context and convention of this library. If @emlautarom1 wants to add the proper @since annotations as requested, this will go in as is.

Lev135 commented 1 year ago

I think that the current state of ecosystem is much more important argument though: suppose you use both mtl and monad-extras: then you have two functions with the same name with different behavior. If you use errors too, than you have two different functions that do the same. All this can be solved by using hiding/qualified imports, but I don't see any reason for such inconveniences here

kozross commented 1 year ago

I agree with @emilypi here. liftMaybe is the right name by convention, makes sense relative the rest of the library, and honestly, qualified imports aren't that bad.

endgame commented 1 year ago

I don't have strong opinions, but I'll drive by and point out that this is one of the instantiations of (<?>) in package hoist-error.