Skip to content

Prepare release 0.2.11.0 #266

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

Merged
merged 1 commit into from
Jun 17, 2020
Merged

Prepare release 0.2.11.0 #266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 22 additions & 3 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,30 @@
## next
## 0.2.11.0

* Add `HashMap.findWithDefault` (soft-deprecates `HashMap.lookupDefault`).
Copy link
Member Author

Choose a reason for hiding this comment

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

Aside: I'm fine with the addition of findWithDefault for compatibility with containers, but I'm wondering whether we really want to get rid of lookupDefault, which is arguably the nicer name.

Thoughts on this, @emilypi, @treeowl, @m-renaud?

The relevant commit is da5ac39.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So, the reason I initially went with findWithDefault is because it had precedence in several packages(hoogle=findWithDefault), including containers which is a "core" package (for some definition of core) as well as nonempty-containers, hashmap, enummapset, and others. lookupDefault also exists in the ecosystem (hoogle=lookupDefault) but fewer, and none in a "core" package.

I don't have a strong preference one way or the other, as long as there is consistency. imho if we decide to go with lookupDefault we should make sure its reflected in all the container-like packages.

Copy link
Collaborator

Choose a reason for hiding this comment

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

As I've said elsewhere, I'd be ever so much happier ignoring or deprecating both of them and using a more "eliminator-style" function like

lurkup :: (Hashable k, Eq k) => r -> (a -> r) -> k -> HashMap k a -> r

Copy link
Collaborator

Choose a reason for hiding this comment

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

lookup = lurkup Nothing Just
lurkup r f k = maybe r f . lookup k

Copy link
Collaborator

@m-renaud m-renaud Jun 6, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

While I agree this is more generally applicable, I have a hunch this will just result in everyone defining differently named versions of (and likely with different argument orderings of) lookupDefault a k m = lurkup a id k m, which will just cause more fragmentation. The idea of "looking up a value from some container, and returning a default value if its missing" is pretty ubiquitous and imo thus deserves a function. If we were to deprecate find/lookupDefault would we then require all code to replace it with a call to lurkup?

Edit: ....or will lead to someone creating a containers-missing package which just wraps containers and exports lookupDefault and any other common operations that are removed from containers to have a more general API.

Copy link
Collaborator

Choose a reason for hiding this comment

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

There's definitely no need to deprecate these things. But it seems strange to me to focus on adding more of them when we haven't yet added the more general idea. Note that while you need to import Data.Maybe to get fromMaybe and Data.Either to get fromLeft and fromRight, maybe and either are right in the Prelude. So is foldr. So this "eliminator style" is even more ubiquitous, and I'd say it's a high priority to get it in here and containers. We just need an appropriate name!

Copy link
Member Author

Choose a reason for hiding this comment

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

I've thought about this some more (and regretted not making a proper issue).

  • I still think the addition of findWithDefault to achieve better consistency with containers is OK.
  • I realized that I do care slightly more about inter-package consistency than perfect naming, so the soft-deprecation of lookupDefault is also OK.
  • If I'd build the first containers-like package for Haskell, I'd probably add neither findWithDefault nor lurkup because both can easily be derived from lookup. These days, I also find that explicit pattern matching helps with readability, so I'd generally recommend using lookup and pattern matching on the result.


* Add `HashMap.fromListWithKey`

* Add `HashMap.findWithDefault` (deprecates `HashMap.lookupDefault`)

* Add more folding functions and use them in `Foldable` instances.

* Add `HashMap.!?`, a flipped version of `lookup`.

* Add `instance Bifoldable HashMap`.

### Bug fixes

* Fix a space leak affecting updates on keys with hash collisions.

* Improvements for `two`: strictness in key arguments, better sharing,
clean up.

### Other changes

* Add call stack to `HashMap.!`.

* Speed up the `Hashable` instances for `HashMap` and `HashSet`.

* Various documentation improvements and fixes.

## 0.2.10.0

* Add `HashMap.alterF`.
Expand Down
6 changes: 4 additions & 2 deletions Data/HashMap/Base.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ instance Foldable.Foldable (HashMap k) where
#endif

#if MIN_VERSION_base(4,10,0)
-- | @since UNRELEASED
-- | @since 0.2.11
instance Bifoldable HashMap where
bifoldMap f g = foldMapWithKey (\ k v -> f k `mappend` g v)
{-# INLINE bifoldMap #-}
Expand Down Expand Up @@ -1251,7 +1251,7 @@ alter f k m =
-- Note: 'alterF' is a flipped version of the 'at' combinator from
-- <https://hackage.haskell.org/package/lens-4.15.4/docs/Control-Lens-At.html#v:at Control.Lens.At>.
--
-- @since 0.2.9
-- @since 0.2.10
alterF :: (Functor f, Eq k, Hashable k)
=> (Maybe v -> f (Maybe v)) -> k -> HashMap k v -> f (HashMap k v)
-- We only calculate the hash once, but unless this is rewritten
Expand Down Expand Up @@ -1945,6 +1945,8 @@ fromListWith f = L.foldl' (\ m (k, v) -> unsafeInsertWith f k v m) empty
--
-- > fromListWith f [(k, a), (k, b), (k, c), (k, d)]
-- > = fromList [(k, f k d (f k c (f k b a)))]
--
-- @since 0.2.11
fromListWithKey :: (Eq k, Hashable k) => (k -> v -> v -> v) -> [(k, v)] -> HashMap k v
fromListWithKey f = L.foldl' (\ m (k, v) -> unsafeInsertWithKey f k v m) empty
{-# INLINE fromListWithKey #-}
Expand Down
4 changes: 3 additions & 1 deletion Data/HashMap/Strict/Base.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ alter f k m =
-- Note: 'alterF' is a flipped version of the 'at' combinator from
-- <https://hackage.haskell.org/package/lens-4.15.4/docs/Control-Lens-At.html#v:at Control.Lens.At>.
--
-- @since 0.2.9
-- @since 0.2.10
alterF :: (Functor f, Eq k, Hashable k)
=> (Maybe v -> f (Maybe v)) -> k -> HashMap k v -> f (HashMap k v)
-- Special care is taken to only calculate the hash once. When we rewrite
Expand Down Expand Up @@ -670,6 +670,8 @@ fromListWith f = L.foldl' (\ m (k, v) -> unsafeInsertWith f k v m) empty
--
-- > fromListWith f [(k, a), (k, b), (k, c), (k, d)]
-- > = fromList [(k, f k d (f k c (f k b a)))]
--
-- @since 0.2.11
fromListWithKey :: (Eq k, Hashable k) => (k -> v -> v -> v) -> [(k, v)] -> HashMap k v
fromListWithKey f = L.foldl' (\ m (k, v) -> unsafeInsertWithKey f k v m) empty
{-# INLINE fromListWithKey #-}
Expand Down
2 changes: 1 addition & 1 deletion unordered-containers.cabal
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: unordered-containers
version: 0.2.10.0
version: 0.2.11.0
synopsis: Efficient hashing-based container types
description:
Efficient hashing-based container types. The containers have been
Expand Down