Skip to content

Fix discrepancy between mergeWithKey impl and docs #1025

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

Merged
merged 1 commit into from
Aug 14, 2024

Conversation

meooow25
Copy link
Contributor

@meooow25 meooow25 commented Aug 13, 2024

The given functions should be called only on nonempty trees.

Fixes #979.

This could alternately be fixed by changing the docs. I find it slightly better to keep the specification that the user will not receive an empty tree.

Copy link
Contributor

@treeowl treeowl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable. Hopefully no one is relying on the current behavior. You're also welcome to make another attempt at function removal, as far as I'm concerned.

@meooow25 meooow25 force-pushed the mergeWithKey-nonempty branch 2 times, most recently from 98c03c9 to 3abd39f Compare August 13, 2024 18:52
@meooow25
Copy link
Contributor Author

Hopefully no one is relying on the current behavior.

I suppose this does cause a change in behavior. Updated the changelog in case someone is affected.

You're also welcome to make another attempt at function removal, as far as I'm concerned.

Personally I'm not too keen on seeing it gone. It is unsafe but can be used properly. I wish it were maybe exported from "Data.Map.Unsafe" or something like that.

Don't call the `only2` function on empty trees.
Fixed for both Map and IntMap.
@meooow25 meooow25 force-pushed the mergeWithKey-nonempty branch from 3abd39f to 298ab3d Compare August 14, 2024 20:11
@meooow25 meooow25 merged commit 734785d into haskell:master Aug 14, 2024
11 checks passed
@meooow25 meooow25 deleted the mergeWithKey-nonempty branch August 14, 2024 20:25
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

Successfully merging this pull request may close these issues.

Data.Map.mergeWithKey doesn't match documentation
2 participants