Add filterKeys for Map and IntMap

[flip111]

#866

[treeowl]

Please don't include your Stack files. While I won't require it, I would prefer to see validity tests and also property tests (with validity tests) added both for filterKeys and filterWithKey. Also, CI is failing.

[flip111]

Hello. The stack files i committed accidentally in my last commit. I will try to fix the test again

[meooow25]

We have

filter p m = filterWithKey (\_ x -> p x) m

So why not

filterKeys p m = filterWithKey (\k _ -> p k) m

instead of duplicating the definition of filterWithKey?

[konsumlamm]

I don't see the advantage of this over just using filterWithKey?

[flip111]

It's the equivalent of filter but then for keys

[konsumlamm]

Sure, I know what it does, but why should we add it, when we can just use filterWithKey, which isn't any slower, instead?

[flip111]

I can't speak for the original poster @emilypi but for my point of view is that haskell libraries have many convenience functions that potentially can be expressed in terms of extra functions. With filterWithKey you have an additional argument that is not used (when using it like filterKeys).

[emilypi]

@flip111 that's exactly it. filterKeys is something many people do, and so we should optimize for it. The point is convenience. It's very low cost to add it, and i would argue it meets my personal Fairbarn threshold due to the fact that every other container library in existence features it.

[konsumlamm]

every other container library in existence features it.

Really? I don't think I've ever seen it anywhere, could you give some examples?

[emilypi]

Some examples:

• guava
• kotlin
• scala - alternatively, view the deprecated version which used to act on the map itself.
• purescript
• clojure - they have a general policy of only supplying O(1) functions in the core libraries (one would have to issue (apply dissoc <f> <map>), but alternative cores supply this like the ones linked.
• C++ experimental - there are many stack overflow questiosn regarding this function, and the current state of the non-experimental art is to write a for-loop which removes keys iteratively using std::map::erase.

There are many more examples, but I don't want to spend too much time on it. I don't see a valid argument against this other than "why not use filterWithKey", which has been addressed, and is largely a subjective for both sides. I've written this function alot, and I'd like to stop the repetition 😄

[PPKFS]

This seems like something that:

• is useful
• is no added overhead
• is not some obscurely named and cryptic to understand function
• sure, it can be done already with filterWithKey or some equivalent but writing filterWithKey (f . snd) feels very strange - or should we be working in favour of removing every function that is just a more specific form of another function?

[Melvar]

I wanted this function a couple weeks ago. Of course I just used filterWithKey, but I had to check for the absence of filterKeys first.

[treeowl]

There seem to be enough people who want this to overcome my general reluctance to add semi-unnecessary functions to this package. I want some documentation and testing tweaks for both versions, even though I only added line comments to one.

[flip111]

@treeowl @emilypi any more comments? I made some changes after your review.

[emilypi]

Looks fine to me, but it's up to @treeowl

[flip111]

@treeowl could you take a look please? Is there anything i can do or something that needs to be done?

[meooow25]

@flip111 would you like to finish the work here?

[flip111]

Sorry i didn't do this earlier, sometimes things seem like a mountain when it's actually not that hard. It went a bit out of focus, thanks for the reminder @meooow25

[flip111]

I guess now, sync with upstream master, rebase and adjust changelog?

[meooow25]

Thanks, just a few more comments and note the test build failure.

I guess now, sync with upstream master, rebase and adjust changelog?

Sure, you can do that if you like.

[meooow25]

The test is broken. It's a small change so I'm just going to go ahead and fix it.

[meooow25]

Alright, I'll squash and merge this soon if there are no other concerns.

[flip111]

Thank you @meooow25

[meooow25]

Thanks for wrapping this up @flip111!