purescript-deprecated/purescript-maps

Add `unionsWith`?

Opened this issue · 10 comments

hdgarrood commented

This function doesn't currently exist, but I think it could, and would potentially be useful:

unionsWith :: forall f k v. (Foldable f, Ord k) => (v -> v -> v) -> f (Map k v) -> Map k v

It's probably very simple to define this using what's already exported, but I think it's still useful to have these sorts of things defined for you.

paf31 commented

👍

Cmdv commented

@paf31 just trying to pick up a ticket to get started with contributing, is this being worked on?

I assume this would need the unionsWith function added then exposed and tests?

  • 👍2
paf31 commented

@Cmdv Yes, that's right, thanks!

Cmdv commented

-- edit: am I wrong in thinking that it is already exposed? here

@paf31 cool I'll give it a go 😄

hdgarrood commented

unionWith is slightly different, it only combines two maps. unionsWith could be used to take the union of an array with any number of maps inside it, for example.

  • 👍1
Cmdv commented

@hdgarrood just got pointed out to me on IRC by garyb as well!! I'll probably get stuck when it comes to testing but I'll crack on :)

garyb commented

For tests, one property to check off the top of my head: the keys of all the maps that were unioned should exist in the resulting map.

garyb commented

Also just to note, we can drop Ord from the signature with the very latest -maps release (it's about 10 minutes old):

unionsWith :: forall f k v. Foldable f => (v -> v -> v) -> f (Map k v) -> Map k v
LiamGoodacre commented

@garyb how would that work??? 😕
With traverse it's fine as we're reconstructing a map in the same way we deconstruct it. But here we're having to combine maps - which means we need to insert the keys in the right place relative to each other.

garyb commented

Uh, yeah, I'm talking nonsense 😆... disregard that.