Couple new foldable functions to Data.Map

[LiamGoodacre]

Added:

• foldableToMap
• foldableToMapWith

Other things:

• Added basic tests for new functions
• Corrected doc comments on some functions mentioning array when they meant list
• Modified Gen import in tests as there was a compiler warning about it
• Docs regenerated (were slightly out of date)

---

These okay?

[LiamGoodacre]

Btw fromList and fromListWith are now special cases of foldableToMap and foldableToMapWith. I switched their implementations instead of removing them because I didn't want to turn this into a breaking change.

Also if you're happy with these, to be consistent we should add StrMap versions too.

[paf31]

What if we just generalize the types of fromList and toList and add fromListWith?

[LiamGoodacre]

I did consider generalizing fromList and fromListWith, however I thought that would be slightly misleading, as they don't necessarily have anything to do with List. But I'm obviously fine with changing them if you think that's okay.

For toList are you thinking:

toList :: forall f k v. (Unfoldable f) => Map k v -> f (Tuple k v)`

[LiamGoodacre]

I've tried writing toList (I called it toUnfoldable) a few times with different forms of intermediate structures: List, Array, CatList, Unfold (newtype tuple around the arguments to unfoldr). The most performant was List.

Whilst doing this I also wrote toMonoid which I thought was interesting, and generalised toList in a different way. It's essentially foldMap except over entries in the Map instead of just the values.

toMonoid :: forall k v m. (Monoid m) => (k -> v -> m) -> Map k v -> m
toMonoid f Leaf = mempty
toMonoid f (Two l k v r) = toMonoid f l <> f k v <> toMonoid f r
toMonoid f (Three l k1 v1 m k2 v2 r) =
  toMonoid f l <> f k1 v1 <> toMonoid f m <> f k2 v2 <> toMonoid f r

toList :: forall k v. Map k v -> List (Tuple k v)
toList = toMonoid (\k v -> Cons (Tuple k v) Nil)

Thoughts about any of this? @paf31 @garyb 😄

[beckyconning]

👍

[beckyconning]

@paf31 @garyb Would this be accepted if it was reduced to the addition of fromFoldable and fromFoldableWith where fromList and fromListWith were synonyms for their foldable counterparts?

[paf31]

Sorry, this has been in my inbox for two weeks and I keep meaning to get round to reviewing it. Thanks for the PR.

It looks good to me, and toMonoid also sounds useful, but I want to understand your analysis some more. I imagine Array would be much faster if you used ST to build up a mutable array first, then froze it all at once. Have you tried that?

[LiamGoodacre]

Ah no I didn't try ST, I'll give it a shot - thanks for the suggestion @paf31 😃
Should I do a separate PR for the unfoldable stuff (toList/toUnfoldable)?

We all seem to be referring to the functions I added by different names. What naming convention should we go by? My thoughts on each that I've seen:

• foldableToMap - can likely be imported unqualified without collisions
• fromFoldable - readable when imported qualified: Map.fromFoldable
• fromList - misleading, doesn't just work with lists

I'd say fromFoldable works better for me, given that Data.Map is one of those modules I tend to always import qualified anyway.

[beckyconning]

I prefer fromFoldable. I have nothing against having a synonym called fromList to prevent braking.

[gbaz]

I was just looking for this too (@soro asked me about it) -- fromFoldable seems the appropriate name.

[hdgarrood]

👍 👍 👍 for fromFoldable. See also the purescript-lists issue I just referenced.

[LiamGoodacre]

I'm currently investigating the toUnfoldable stuff; which I'll do separately. Even using STArray appears slower than List - I will make a new branch/PR with my testing so people can look at it and make sure I'm not doing crazy things 😄. In doing this I also found a performance hit in pushSTArray which I'll make a PR for.

As for the Foldable stuff, how's this looking now? @paf31

[paf31]

Looks good to me! Thanks, and sorry for the delay. Nothing is a breaking change here, right?

[paf31]

👍

[paf31]

Tagged as 0.5.4

[LiamGoodacre]

Thanks!

[hdgarrood]

Hooray :)

Incidentally, I like the idea of toMonoid too, although I think it might be better named foldMapWithKey.

[LiamGoodacre]

@hdgarrood just been playing around with a class for this: https://github.com/LiamGoodacre/purescript-twofoldable/blob/master/docs/Data/Twofoldable.md

[paf31]

@LiamGoodacre Have you seen IndexedTraversal from lens?

[LiamGoodacre]

This guy?

type IndexedTraversal i s t a b =
  forall p f. (Indexable i p, Applicative f) => p a (f b) -> s -> f t 

http://haddock.stackage.org/lts-3.18/lens-4.12.3/Control-Lens-Type.html#t:IndexedTraversal

[paf31]

Yes. I don't fully understand it yet, but it seems relevant to these sorts of functions where you're folding/traversing with a key or index.

[LiamGoodacre]

Just found this too:
http://haddock.stackage.org/lts-3.18/lens-4.12.3/Control-Lens-Fold.html#v:ifoldMapOf

ifoldMapOf ::             IndexedGetting i m s a -> (i -> a -> m) -> s -> m
ifoldMapOf :: Monoid m => IndexedFold i s a      -> (i -> a -> m) -> s -> m

-- version where I've specialised to Map:
ifoldMapOf :: Monoid m => IndexedFold k (Map k v) v -> (k -> v -> m) -> Map k v -> m

And then this:
http://haddock.stackage.org/lts-3.18/lens-4.12.3/Control-Lens-Indexed.html#v:ifoldMap

class Foldable f => FoldableWithIndex i f | f -> i where
  ifoldMap :: Monoid m => (i -> a -> m) -> f a -> m
  -- ...

[paf31]

Perhaps the most general thing which can be exported is an indexed traversal, then? (No lens import required, of course, just using Applicative). It wouldn't be very fast or stack-safe though.