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

Added liftMaybe for lifting Maybe into MonadError #137

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

emlautarom1
Copy link

On #37 it was discussed the existence of liftMaybe:

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

Originally posted by @ocharles in #37 (comment)

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
Copy link

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
Copy link
Author

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
Copy link

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
Copy link
Member

emilypi commented Dec 15, 2022

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
Copy link
Collaborator

kozross commented Dec 15, 2022

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

@emilypi
Copy link
Member

emilypi commented Dec 15, 2022

@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
Copy link
Author

emlautarom1 commented Dec 16, 2022

There are multiple liftMaybes in the wild:

  • extensible-effects has liftMaybe :: Member Fail r => Maybe a -> Eff r a which just throws ().
  • Agda has liftMaybe :: Alternative f => Maybe a -> f a which uses empty on Nothing
  • monad-extras has liftMaybe :: MonadPlus m => Maybe a -> m a which uses mzero on Nothing

note has only two common interpretations:

  • errors has note :: a -> Maybe b -> Either a b, which uses a provided value on Nothing
  • protolude, universum and polysemy all have some form of note :: MonadError e m => e -> Maybe a -> m a, which is the proposed liftMaybe

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
Copy link

Lev135 commented Jun 21, 2023

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
Copy link
Member

emilypi commented Jun 21, 2023

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
Copy link

Lev135 commented Jun 22, 2023

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
Copy link
Collaborator

kozross commented Jun 22, 2023

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
Copy link
Contributor

endgame commented Jun 23, 2023

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.

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