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

Proposal: add Dict.intersectBoth #402

Closed
mgold opened this Issue Sep 9, 2015 · 3 comments

Comments

Projects
None yet
2 participants
@mgold
Contributor

mgold commented Sep 9, 2015

I recently found myself writing this code:

let commonKeys = Dict.intersect dict1 dict2 |> Dict.keys
    unsafeGet k d = Dict.get k d |> (\(Just x) -> x)
in List.map (\k -> (unsafeGet k dict1, unsafeGet k dict2)) commonKeys

This requires the unsafe (\(Just x) -> x) operation, even though I know it's safe because I'm only looking at keys in the intersection. Although, I was using the union at first, and that blew up in a runtime error, so this is a real runtime error that we can avoid by offering this API in Dict:

intersectBoth : Dict comparable v -> Dict.comparable w -> Dict.comparable (v,w)

The current intersect function keeps only the values from the first dictionary, even though we know they're also in the second one (because it's the intersection). It's pretty useless in my case:

intersect : Dict comparable v -> Dict comparable v -> Dict comparable v

A more extreme change would be to replace it with the function I've called intersectBoth, since you can get back the old behavior with Dict.map (always fst). But if you don't want to do that, adding another function shouldn't be too bad. Because the representation of dictionaries is opaque, this can't be done in a third party library.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Oct 2, 2015

Member

I am a fan. I'd want it to be something like this:

intersectWith : (a -> b -> c) -> Dict comparable a -> Dict comparable b -> Dict comparable c

So you could do intersect = intersectWith always and intersectBoth = intersectWith (,). I'm not certain about switching the meaning of intersect but I'm doing some core stuff now, so I'll think about this more.

Member

evancz commented Oct 2, 2015

I am a fan. I'd want it to be something like this:

intersectWith : (a -> b -> c) -> Dict comparable a -> Dict comparable b -> Dict comparable c

So you could do intersect = intersectWith always and intersectBoth = intersectWith (,). I'm not certain about switching the meaning of intersect but I'm doing some core stuff now, so I'll think about this more.

@evancz evancz referenced this issue Oct 2, 2015

Open

API Ideas #322

@mgold

This comment has been minimized.

Show comment
Hide comment
@mgold

mgold Oct 2, 2015

Contributor

I can live with intersectWith. Thanks.

Contributor

mgold commented Oct 2, 2015

I can live with intersectWith. Thanks.

@evancz evancz closed this Nov 30, 2015

evancz pushed a commit that referenced this issue Mar 1, 2016

Add Dict.unionWith and Dict.intersectWith
Suggested in #402 and needed in some code I was writing recently
@mgold

This comment has been minimized.

Show comment
Hide comment
@mgold

mgold Jun 28, 2016

Contributor

I don't think the commit referenced above was ever released, but it's covered by Dict.merge in 4.0.1.

Contributor

mgold commented Jun 28, 2016

I don't think the commit referenced above was ever released, but it's covered by Dict.merge in 4.0.1.

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