Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add filterKeys for Map and IntMap #972

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add filterKeys for Map and IntMap #972

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f030760
Add filterKeys for Map and IntMap
flip111 Sep 23, 2023
c8a992f
Fix test
flip111 Sep 23, 2023
faab8f7
Fix test, remove stack files
flip111 Sep 30, 2023
5e14d30
Using filterWithKey for filterKeys
flip111 Oct 2, 2023
1c8ebc2
Changes after review
flip111 Jan 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions containers-tests/tests/intmap-properties.hs
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

We seem to be missing property tests for filterWithKey and filter as well as the new function. The one for filterWithKey should look something like this:

prop_filterWithKey :: Fun (Int, Int) Bool -> IMap -> Property
prop_filterWithKey fun m =
  valid m' .&&. toList m' === Prelude.filter (apply fun) (toList m)
  where
    m' = filterWithKey (apply2 fun) m

The others should be similar, with appropriate tweaks.

Copy link
Member

@emilypi emilypi Jan 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

If we implement filterKeys in terms of filterWithKey, that'd be great, but if we end up with the duplicate definition we currently have, I'd also want some tests for fidelity between the two a la

prop_filterKeysFidelity :: Fun Int Bool -> IMap -> Property
prop_filterKeysFidelity p m = fwk === fk 
  where
    fwk = filterWithKey (\k _ -> apply p k) m
    fk = filterKeys (apply p) m

Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ main = defaultMain $ testGroup "intmap-properties"
, testCase "fromAscListWithKey" test_fromAscListWithKey
, testCase "fromDistinctAscList" test_fromDistinctAscList
, testCase "filter" test_filter
, testCase "filterWithKey" test_filteWithKey
, testCase "filterKeys" test_filterKeys
, testCase "filterWithKey" test_filterWithKey
, testCase "partition" test_partition
, testCase "partitionWithKey" test_partitionWithKey
, testCase "mapMaybe" test_mapMaybe
Expand Down Expand Up @@ -858,8 +859,13 @@ test_filter = do
filter (> "x") (fromList [(5,"a"), (-3,"b")]) @?= empty
filter (< "a") (fromList [(5,"a"), (-3,"b")]) @?= empty

test_filteWithKey :: Assertion
test_filteWithKey = do
test_filterKeys :: Assertion
test_filterKeys = do
filterKeys (> 4) (fromList [(5,"a"), (3,"b")]) @?= singleton 5 "a"
filterKeys (> 4) (fromList [(5,"a"), (-3,"b")]) @?= singleton 5 "a"

test_filterWithKey :: Assertion
test_filterWithKey = do
filterWithKey (\k _ -> k > 4) (fromList [(5,"a"), (3,"b")]) @?= singleton 5 "a"
filterWithKey (\k _ -> k > 4) (fromList [(5,"a"), (-3,"b")]) @?= singleton 5 "a"

Expand Down
9 changes: 6 additions & 3 deletions containers-tests/tests/map-properties.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ main = defaultMain $ testGroup "map-properties"
, testCase "fromDistinctAscList" test_fromDistinctAscList
, testCase "fromDistinctDescList" test_fromDistinctDescList
, testCase "filter" test_filter
, testCase "filterWithKey" test_filteWithKey
, testCase "filterWithKey" test_filterWithKey
, testCase "partition" test_partition
, testCase "partitionWithKey" test_partitionWithKey
, testCase "mapMaybe" test_mapMaybe
Expand Down Expand Up @@ -820,8 +820,11 @@ test_filter = do
filter (> "x") (fromList [(5,"a"), (3,"b")]) @?= empty
filter (< "a") (fromList [(5,"a"), (3,"b")]) @?= empty

test_filteWithKey :: Assertion
test_filteWithKey = filterWithKey (\k _ -> k > 4) (fromList [(5,"a"), (3,"b")]) @?= singleton 5 "a"
test_filterKeys :: Assertion
test_filterKeys = filterKeys (> 4) (fromList [(5,"a"), (3,"b")]) @?= singleton 5 "a"
Comment on lines +823 to +824
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not used, it needs to be added to the list of tests above.

, testCase "filterKeys" test_filterKeys

Also, it would be good to have property tests (like for IntMap).


test_filterWithKey :: Assertion
test_filterWithKey = filterWithKey (\k _ -> k > 4) (fromList [(5,"a"), (3,"b")]) @?= singleton 5 "a"

test_partition :: Assertion
test_partition = do
Expand Down
12 changes: 12 additions & 0 deletions containers/src/Data/IntMap/Internal.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ module Data.IntMap.Internal (

-- * Filter
, filter
, filterKeys
, filterWithKey
, restrictKeys
, withoutKeys
Expand Down Expand Up @@ -2578,6 +2579,17 @@ filter :: (a -> Bool) -> IntMap a -> IntMap a
filter p m
= filterWithKey (\_ x -> p x) m

-- | \(O(n)\). Filter all keys that satisfy some predicate.
--
-- > filterKeys (> 4) (fromList [(5,"a"), (3,"b")]) == singleton 5 "a"
flip111 marked this conversation as resolved.
Show resolved Hide resolved

filterKeys :: (Key -> Bool) -> IntMap a -> IntMap a
filterKeys predicate = go
where
go Nil = Nil
go t@(Tip k _) = if predicate k then t else Nil
go (Bin p m l r) = bin p m (go l) (go r)
flip111 marked this conversation as resolved.
Show resolved Hide resolved

-- | \(O(n)\). Filter all keys\/values that satisfy some predicate.
--
-- > filterWithKey (\k _ -> k > 4) (fromList [(5,"a"), (3,"b")]) == singleton 5 "a"
Expand Down
1 change: 1 addition & 0 deletions containers/src/Data/IntMap/Lazy.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ module Data.IntMap.Lazy (

-- * Filter
, IM.filter
, filterKeys
, filterWithKey
, restrictKeys
, withoutKeys
Expand Down
1 change: 1 addition & 0 deletions containers/src/Data/IntMap/Strict.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ module Data.IntMap.Strict (

-- * Filter
, filter
, filterKeys
, filterWithKey
, restrictKeys
, withoutKeys
Expand Down
2 changes: 2 additions & 0 deletions containers/src/Data/IntMap/Strict/Internal.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ module Data.IntMap.Strict.Internal (

-- * Filter
, filter
, filterKeys
, filterWithKey
, restrictKeys
, withoutKeys
Expand Down Expand Up @@ -286,6 +287,7 @@ import Data.IntMap.Internal
, empty
, assocs
, filter
, filterKeys
, filterWithKey
, findMin
, findMax
Expand Down
8 changes: 8 additions & 0 deletions containers/src/Data/Map/Internal.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ module Data.Map.Internal (

-- * Filter
, filter
, filterKeys
, filterWithKey

, takeWhileAntitone
Expand Down Expand Up @@ -2857,6 +2858,13 @@ filter :: (a -> Bool) -> Map k a -> Map k a
filter p m
= filterWithKey (\_ x -> p x) m

-- | \(O(n)\). Filter all keys that satisfy the predicate.
--
-- > filterKeys (> 4) (fromList [(5,"a"), (3,"b")]) == singleton 5 "a"

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a @since FIXME at the end of the docs. It will be updated by maintainers to the next release version.
Same for IntMap.

filterKeys :: (k -> Bool) -> Map k a -> Map k a
filterKeys p m = filterWithKey (\k _ -> p k) m

-- | \(O(n)\). Filter all keys\/values that satisfy the predicate.
--
-- > filterWithKey (\k _ -> k > 4) (fromList [(5,"a"), (3,"b")]) == singleton 5 "a"
Expand Down
1 change: 1 addition & 0 deletions containers/src/Data/Map/Lazy.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ module Data.Map.Lazy (

-- * Filter
, filter
, filterKeys
, filterWithKey
, restrictKeys
, withoutKeys
Expand Down
1 change: 1 addition & 0 deletions containers/src/Data/Map/Strict.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ module Data.Map.Strict

-- * Filter
, filter
, filterKeys
, filterWithKey
, restrictKeys
, withoutKeys
Expand Down
2 changes: 2 additions & 0 deletions containers/src/Data/Map/Strict/Internal.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ module Data.Map.Strict.Internal

-- * Filter
, filter
, filterKeys
, filterWithKey
, restrictKeys
, withoutKeys
Expand Down Expand Up @@ -359,6 +360,7 @@ import Data.Map.Internal
, drop
, dropWhileAntitone
, filter
, filterKeys
, filterWithKey
, findIndex
, findMax
Expand Down