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

port Haskell partitionEithers #38

Closed
matthewleon opened this issue Apr 21, 2018 · 12 comments
Closed

port Haskell partitionEithers #38

matthewleon opened this issue Apr 21, 2018 · 12 comments

Comments

@matthewleon
Copy link
Contributor

matthewleon commented Apr 21, 2018

from base (https://hackage.haskell.org/package/base-4.11.1.0/docs/Data-Either.html):

partitionEithers :: [Either a b] -> ([a], [b])

suggested PS equivalent:

partition :: forall f a b. Foldable f => f (Either a b) -> Tuple (f a) (f b)
@matthewleon matthewleon changed the title port Haskell partitionEither port Haskell partitionEithers Apr 21, 2018
@matthewleon
Copy link
Contributor Author

Closing this as I don't see a good way to do it generically.

@natefaubion
Copy link
Contributor

partition :: forall f g a b. Foldable f => Unfoldable g => f (Either a b) -> Tuple (g a) (g b)

?

@natefaubion
Copy link
Contributor

It would require two traversals, granted.

@natefaubion
Copy link
Contributor

Or use Filterable https://pursuit.purescript.org/packages/purescript-filterable/2.4.1/docs/Data.Filterable#v:partitionMap

@matthewleon
Copy link
Contributor Author

Yeah, that does work. What do people think of adding both tuples and unfoldable as a dep for this though? Might not be worth it.

@matthewleon matthewleon reopened this Apr 21, 2018
@matthewleon
Copy link
Contributor Author

Or use Filterable

Thanks for this. Will do it.

@JordanMartinez
Copy link
Contributor

I think this should be added, but I'm hesitant because of the dependencies one would need to add.

@bbarker
Copy link

bbarker commented Jan 6, 2021

An alternative that I came up with based on catMaybes:

catLefts ::  forall a b m. Monad m => Plus m => m (Either a b) -> m a
catLefts = catMapLefts identity
catMapLefts :: forall a b c m. Monad m => Plus m
  => (a -> c) -> m (Either a b) -> m c
catMapLefts f = (flip bind) (leftOr empty (pure <<< f))
leftOr :: forall a b c. c -> (a -> c) -> Either a b -> c
leftOr _ f (Left a) = f a
leftOr c _ (Right _) = c

I don't think this requires any additional dependencies on this package - correct me if I'm wrong.

Would a PR with this (and the related Right-versions) be welcome here? I have a need to use this in a few repositories at the moment, thus the desire to get it extracted into a library.

@hdgarrood
Copy link
Contributor

I would be concerned about the performance of that function, and I’d also worry that it might not behave performantly or even sensibly for some types which don’t have append-y Plus instances (like Effect). I think I’d be unlikely to approve adding that to core; the Filterable formulation is more appealing to me.

In general, I think things which haven’t already been tested and found to be effective and useful in practice shouldn’t go straight into core. This is because it’s much more expensive to change stuff once it’s in core since so many more people will he depending on it, and because enlarging core puts more maintenance burden on the core team. I’d suggest putting them in your own library for now (or switching to Filterable).

@bbarker
Copy link

bbarker commented Jan 6, 2021

Thanks! The performance didn't look great to me either, though I hadn't thought of Effect having a Plus instance. I mainly went this way as concatMap isn't part of a class.

@JordanMartinez
Copy link
Contributor

Closing as Filterable enables this.

@thomashoneyman
Copy link
Member

Closing as Filterable enables this.

(And is now a core library, which wasn’t previously true!)

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

6 participants