<div dir="ltr">Reviving this thread since we didn't reach a definitive conclusion (and its been about a year!).<div><br></div><div>It appears that the PVP issue about the deprecation has not yet been resolved, so there still isn't any clarification on if this would require a major version bump or not. From re-reading the thread it appears the largest (only?) concerns are around how to properly deprecate it without breaking anyone's code that is compiled with -Werror.</div><div><br></div><div>As a partial solution, I propose that we add the new HashMap.findWithDefault function (which brings the APIs between Map and HashMap closer together), and keep the existing function (HashMap.lookupDefault) but update the function comment to say that it is deprecated and point to findWithDefault. Are there any objections? We should eventually officially mark this function as deprecated but I don't think we should block the addition of the alias in the meantime because there's not currently consensus on how it should happen, I'd like to move that to a separate discussion.</div><div><br></div><div>Thanks! </div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Jan 29, 2018 at 6:09 PM Matt Renaud <<a href="mailto:matt@m-renaud.com">matt@m-renaud.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">> <span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">I'm wondering if we should ultimately get rid of that function altogether, from both packages</span><div><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br></span></div><div><span style="font-size:12.8px">I'm on the fence about this one since its such a common operation, I personally would be a little surprised if it wasn't part of the API. You're right that it's not /that/ much code to write, but I imagine everyone will end up writing their own findWithDefault in their codebase, I may be wrong though.</span></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 24, 2018 at 4:27 PM, David Feuer <span dir="ltr"><<a href="mailto:david.feuer@gmail.com" target="_blank">david.feuer@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="auto">I'm wondering if we should ultimately get rid of that function altogether, from both packages. From an API design standpoint, it's kind of silly, because<div dir="auto"><br></div><div dir="auto"> findWithDefault d k m =</div><div dir="auto"> fromMaybe d (lookup k m)</div><div dir="auto"><br></div><div dir="auto">At present, there's a slight performance penalty to doing it that way. I wonder if we can squash that now that GHC has unboxed sums.</div><div dir="auto"><br></div><div dir="auto"> lookup# :: ... => k -> m a -> (# (# #) | a #)</div><div dir="auto"> lookup k m = case lookup# k m of</div><div dir="auto"> (# _ | #) -> Nothing</div><div dir="auto"> (# | a #) -> Just a</div><div dir="auto"><br></div><div dir="auto">Now case-of-case will get rid of the extra Maybe.</div><br><div class="gmail_extra" dir="auto"><br><div class="gmail_quote"><div><div class="gmail-m_-1356821413805320934h5">On Jan 23, 2018 9:15 PM, "Matt Renaud" <<a href="mailto:matt@m-renaud.com" target="_blank">matt@m-renaud.com</a>> wrote:<br type="attribution"></div></div><blockquote class="gmail-m_-1356821413805320934m_-3818334782609928647quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="gmail-m_-1356821413805320934h5"><div dir="ltr"><b>What</b><div><b>====<br></b><div><br></div><div>Rename unordered-container's <a href="https://hackage.haskell.org/package/unordered-containers-0.2.8.0/docs/Data-HashMap-Strict.html#v:lookupDefault" target="_blank">Data.HashMap.lookupDefault</a> to findWithDefault.</div><div><br></div><div>findWithDefault :: (Eq k, Hashable k) => v -> k -> HashMap k v -> v</div><div><br></div><div>Note: There are no functionality changes, this is purely a rename.</div><div><br></div><div><br></div><div><b>Why</b></div><div><b>===</b></div><div><br></div><div>This aligns the Data.HashMap API with containers' <a href="https://hackage.haskell.org/package/containers-0.5.11.0/docs/Data-Map-Strict.html#v:findWithDefault" target="_blank">Data.Map.findWithDefault</a>.</div><div><br></div><div>Data.Map.findWithDefault :: Ord k => a -> k -> Map k a -> a</div><div><br></div><div>The map APIs provided by the two packages are <i>almost</i> drop in replacement compatible if you aren't using any of the implementation specific functions (like ordering based functions from Data.Map), this change brings us one step closer.</div><div><br></div><div>API consistency reduces the cognitive overhead when learning a new package. Having to learn different function names for the same functionality depending on which "map" implementation you're using is a poor developer experience.</div><div><br></div><div>We chose the containers' name findWithDefault over unordered-containers' lookupDefault for two reasons:</div><div><ol><li>Existing lookupX functions returns a Maybe value, while findX functions return a non-wrapped value.</li><li>The containers package ships with GHC and is a "core" package.</li></ol></div><div><br></div><div><i>Pros:</i></div><div><i>-----</i></div><div><i><br></i></div><div>- Consistent API between different "map" implementations (Data.Map, Data.IntMap, Data.HashMap). This makes switching implementations an import change.</div><div>- Naming matches other similar functions (lookupX return Maybe-wrapped values)</div><div><br></div><div><i>Cons:</i></div><div><i>-----</i></div><div><i><br></i></div><div>- API change requires users to update their code</div><div> + unordered-containers has A LOT of users: 358815 total (13325 in the last 30 days)</div><div><br></div><div><br></div><div><b>How</b></div><div><b>===</b></div><div><br></div><div><a href="https://github.com/tibbe/unordered-containers/pull/176/commits/152f8818ee13dacb370e49b904edc4c1a4c8f87b" target="_blank">https://github.com/tibbe/unordered-containers/pull/176/commits/152f8818ee13dacb370e49b904edc4c1a4c8f87b</a><br></div><div><br></div><div><i>Code Changes:</i></div><div><i>-------------</i></div><div><i><br></i></div><div>- Rename the function in Data.HashMap.Base (and expose it from Strict and Lazy modules)</div><div>- Make lookupDefault an INLINE alias of findWithDefault</div><div>- Add DEPRECATION notice to lookupDefault</div><div>- Bump unordered-containers version to 0.2.9.0</div><div><br></div><div><br></div><div><i>Migration - Option 1:</i></div><div><i>---------------------<br></i></div><div><i><br></i></div><div><i>- </i>Announce on Haskell communication channels (haskell-cafe@, haskell-community@, #haskell on Twitter, Reddit thread, etc.)</div><div>- Users of unordered-containers >= 0.2.9.0 receive warning about deprecated function</div><div>- Code can be updated by find and replace: s/lookupDefault/findWithDefault/</div><div>- lookupDefault with deprecation notice remains for 1 year (subject to change)</div><div>- after 1 year the lookupDefault function is removed, unordered-containers version bumped to 0.3.0.0 (major version bump due to breaking change)</div><div><br></div><div><i>Migration - Option 2:</i></div><div><i>---------------------<br></i></div><div><i><br></i></div><div>- Announce on Haskell communication channels (haskell-cafe@, haskell-community@, #haskell on Twitter, Reddit thread, etc.)<i><br></i></div><div><div>- Users of unordered-containers >= 0.2.9.0 receive warning about deprecated function</div><div>- Code can be updated by find and replace: s/lookupDefault/findWithDefault/</div></div><div>- lookupDefault function is never removed</div><div><i><br></i></div><div><br></div><div><div><b>Discussion: </b></div></div><div><b>===========</b></div><div><b><br></b></div><div><div>I would like to get some comments on this proposal. In particular: </div><div>- Is the small API churn worth the increase in consistency?</div></div><div>- Should migration option 1 (completely remove the old function) or 2 (keep old function indefinitely) be taken? We can punt on this and go with option 2 to start and revisit later if desired.</div><div><br></div><div><div>I hope a decision about the proposal can be reached by 2018-02-09. Thanks!</div></div><div><br></div><div><br></div><div><br></div><div><br></div></div></div>
<br></div></div><span>_______________________________________________<br>
Libraries mailing list<br>
<a href="mailto:Libraries@haskell.org" target="_blank">Libraries@haskell.org</a><br>
<a href="http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries" rel="noreferrer" target="_blank">http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries</a><br>
<br></span></blockquote></div><br></div></div>
</blockquote></div><br></div>
</blockquote></div>