Skip to content

Add Data.Set.catMaybes and Data.Set.mapMaybe? #346

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

Open
sjakobi opened this issue Oct 6, 2016 · 19 comments
Open

Add Data.Set.catMaybes and Data.Set.mapMaybe? #346

sjakobi opened this issue Oct 6, 2016 · 19 comments

Comments

@sjakobi
Copy link
Member

sjakobi commented Oct 6, 2016

The types should be the following:

catMaybes :: Ord a => Set (Maybe a) -> Set a
mapMaybe :: Ord b => (a -> Maybe b) -> Set a -> Set b

Currently I often use Set.fromList . mapMaybe f . Set.toList which I find rather tedious.

@mitchellwrosen
Copy link

Hm,

catMaybes = filter isJust
mapMaybe f = filter isJust . map f

these are a little shorter than what you've written, and don't seem too bad.

@sjakobi
Copy link
Member Author

sjakobi commented Nov 12, 2016

catMaybes = filter isJust
mapMaybe f = filter isJust . map f

These would have the wrong result type (Set (Maybe a)), no?

@mitchellwrosen
Copy link

Ah - yup! Blah. Carry on then

@treeowl
Copy link
Contributor

treeowl commented Nov 12, 2016

Can you explain more about when you need these? I'm not opposed, but would
like more information.

catMaybes can be implemented efficiently using the current API:

catMaybes xs = case minView xs of
Nothing -> empty
Just (Nothing, xs') -> mapMonotonic fromJust xs'
_ -> mapMonotonic fromJust xs

However, it's not clear to me why you would build a Set of Maybes to begin
with. Set (Maybe a) ~= (Bool, Set a), but the pair representation will be
more compact and usually faster when optimized.

I don't know of a way to implement mapMaybe much (if at all) better than
your version.

On Oct 6, 2016 5:10 AM, "Simon Jakobi" [email protected] wrote:

The types should be the following:

catMaybes :: Ord a => Set (Maybe a) -> Set a
mapMaybe :: Ord b => (a -> Maybe b) -> Set a -> Set b

Currently I often use Set.fromList . mapMaybe f . Set.toList which I find
rather tedious.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#346, or mute the thread
https://github.com/notifications/unsubscribe-auth/ABzi_T8F05uU0XGYal68rWWbjud7H_hxks5qxLsdgaJpZM4KPtm5
.

@treeowl
Copy link
Contributor

treeowl commented Nov 13, 2016

I found a simpler version of catMaybes, using a function new in the latest
version of containers:

catMaybes = mapMonotonic fromJust . dropWhileAntitone isNothing

On Nov 12, 2016 6:52 PM, "David Feuer" [email protected] wrote:

Can you explain more about when you need these? I'm not opposed, but would
like more information.

catMaybes can be implemented efficiently using the current API:

catMaybes xs = case minView xs of
Nothing -> empty
Just (Nothing, xs') -> mapMonotonic fromJust xs'
_ -> mapMonotonic fromJust xs

However, it's not clear to me why you would build a Set of Maybes to begin
with. Set (Maybe a) ~= (Bool, Set a), but the pair representation will be
more compact and usually faster when optimized.

I don't know of a way to implement mapMaybe much (if at all) better than
your version.

On Oct 6, 2016 5:10 AM, "Simon Jakobi" [email protected] wrote:

The types should be the following:

catMaybes :: Ord a => Set (Maybe a) -> Set a
mapMaybe :: Ord b => (a -> Maybe b) -> Set a -> Set b

Currently I often use Set.fromList . mapMaybe f . Set.toList which I
find rather tedious.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#346, or mute the thread
https://github.com/notifications/unsubscribe-auth/ABzi_T8F05uU0XGYal68rWWbjud7H_hxks5qxLsdgaJpZM4KPtm5
.

@sjakobi
Copy link
Member Author

sjakobi commented Nov 13, 2016

Can you explain more about when you need these?

This version of mapMaybe appears a few times in stack, e.g. here, here and here.

Regarding catMaybes I think I added it in my initial request to be consistent with [], even though I can't remember ever using it. As you explained, it probably wouldn't be very useful, so I retract my request for adding it.

@mitchellwrosen
Copy link

mitchellwrosen commented Jun 9, 2018

Also missing:

Data.Sequence.mapMaybe :: (a -> Maybe b) -> Seq a -> Seq b

@ocharles
Copy link

I just found myself wanting mapMaybe. Even if there's no efficient way to implement it other than round-tripping through a list, it provides a nicer API for users, and is consistent with Data.Map (which has mapMaybe).

@srid
Copy link

srid commented Apr 30, 2020

Wouldn't it be better if Set was made an instance Filterable? Though that will introduce a new dependency: the witherable package.

@treeowl
Copy link
Contributor

treeowl commented Apr 30, 2020

Wouldn't it be better if Set was made an instance Filterable? Though that will introduce a new dependency: the witherable package.

Set can't be, because it's not a Functor. Map k already is (the instance is defined in the witherable package). Side note: containers has to be extremely conservative about dependencies because it's an exposed GHC boot package.

@isovector
Copy link

I just spent a few minutes searching for Data.Set.mapMaybe. Please add it.

@treeowl
Copy link
Contributor

treeowl commented Oct 30, 2020

My concern remains: mapMaybe is O(n log n). Users tend to assume O(n). It it worth adding?

@isovector
Copy link

Add a comment?

@treeowl
Copy link
Contributor

treeowl commented Oct 30, 2020

Okay, fine.

@meooow25
Copy link
Contributor

meooow25 commented Mar 15, 2025

Sounds good to add mapMaybe to Set, IntSet, Seq.
It shouldn't be hard to adopt Set's map into a mayMaybe. Same for IntSet.
For Seq, it seems easier to adopt filter.

(btw filter looks more strict than I would expect, but that's a different matter)

@treeowl
Copy link
Contributor

treeowl commented Mar 15, 2025

Recovering mapMaybe from filter for Seq wouldn't be very nice at all, IMO. I don't think we can do anything terribly clever; something like this is likely best:

mapMaybe :: (a -> Maybe b) -> Seq a -> Seq b
mapMaybe f = fromList . Data.Maybe.mapMaybe f . toList

@meooow25
Copy link
Contributor

I think it's reasonable to expect filter and mapMaybe to have similar behavior.
It's possible it would be better to have filter = fromList . List.filter f . toList instead, what do you think?

@treeowl
Copy link
Contributor

treeowl commented Mar 15, 2025

It's worth checking both options. The fold is perfectly reasonable too, for both of them. What's not reasonable is using filter to define mapMaybe.

@meooow25
Copy link
Contributor

Oh of course, I guess this was a misunderstanding. I suggested defining mapMaybe in the same style as filter, not using filter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants