Skip to content

Add unionsWith to combine a list of hash maps#118

Draft
ppetr wants to merge 2 commits into
haskell-unordered-containers:masterfrom
ppetr:master
Draft

Add unionsWith to combine a list of hash maps#118
ppetr wants to merge 2 commits into
haskell-unordered-containers:masterfrom
ppetr:master

Conversation

@ppetr

@ppetr ppetr commented Jan 22, 2016

Copy link
Copy Markdown

No description provided.

@sjakobi sjakobi linked an issue May 31, 2020 that may be closed by this pull request
@sjakobi

sjakobi commented May 31, 2020

Copy link
Copy Markdown
Member

Uh, sorry for the late review… 😄

Probably a dumb question, by why doesn't this need both strict and lazy variants?

@sjakobi sjakobi mentioned this pull request Jun 18, 2020
@sjakobi

sjakobi commented Jun 18, 2020

Copy link
Copy Markdown
Member

Probably a dumb question, but why doesn't this need both strict and lazy variants?

To answer my question: It quite certainly does! We also have lazy and strict variants of unionWith.

Before any further work on the code is done here, we need to make a decision on the correct type for this function though: See #240 (comment).

@treeowl

treeowl commented Jun 18, 2020

Copy link
Copy Markdown
Collaborator

I continue to wonder if this function meets the Fairbairn threshold. It's an entirely immediate application of foldl' and unionWith.

@sjakobi

sjakobi commented Jun 18, 2020

Copy link
Copy Markdown
Member

I continue to wonder if this function meets the Fairbairn threshold. It's an entirely immediate application of foldl' and unionWith.

One argument for providing the function is that we could include proper documentation on the order in which the values are combined. unionWith unionsWith has the same problem as fromListWith, I believe.

@treeowl

treeowl commented Jun 18, 2020

Copy link
Copy Markdown
Collaborator

That's a problem for unionWith. If we don't provide unionsWith, then we certainly don't have to document it! At all! Huge win.

@sjakobi

sjakobi commented Jun 18, 2020

Copy link
Copy Markdown
Member

Sorry, I meant to say that unionsWith has a similar gotcha as fromListWith. That's an argument for holding off on unionsWith until we've made some progress on #264 though.

@sjakobi sjakobi removed a link to an issue Jun 20, 2020
@sjakobi sjakobi marked this pull request as draft June 24, 2020 10:58
@Prillan

Prillan commented Dec 29, 2022

Copy link
Copy Markdown

I continue to wonder if this function meets the Fairbairn threshold. It's an entirely immediate application of foldl' and unionWith.

Just a comment from a random passerby, I'd much prefer to have unions and unionsWith exposed by the library since I, a normal user without any intricate knowledge of the library, do not know which implementations would be best.

By not including the two functions you would leave it up to everyone to figure out whether foldl' union empty, fromList . concatMap toList or any other version is the most performant. The best implementation might not even use any exported functions!

Comment thread Data/HashMap/Base.hs Outdated
…'maps', not 'sets'.

Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com>
@sjakobi sjakobi added the compatibility-with-containers API compatibility with the containers package label Oct 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

compatibility-with-containers API compatibility with the containers package feature-request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants