<div dir="ltr">But, I think that the problem with adding `__hash__` to collections.abc.Iterable is that not all iterables are immutable -- And if they aren't immutable, then allowing them to be hashed is likely to be a pretty bad idea...<div><br></div><div>I'm still having a hard time being convinced that this is very much of an optimization at all ...</div><div><br></div><div>If you start hashing tuples that are large enough that memory is a concern, then that's going to also take a <i>really</i> long time and probably be prohibitive anyway.  Just for kicks, I decided to throw together a simple script to time how much penalty you pay for hashing a tuple:</div><div><br></div><div><div><font face="monospace, monospace">class F(object):</font></div><div><font face="monospace, monospace">    def __init__(self, arg):</font></div><div><font face="monospace, monospace">        self.arg = arg</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace">    def __hash__(self):</font></div><div><font face="monospace, monospace">        return hash(tuple(self.arg))</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace">class T(object):</font></div><div><font face="monospace, monospace">    def __init__(self, arg):</font></div><div><font face="monospace, monospace">        self.arg = tuple(arg)</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace">    def __hash__(self):</font></div><div><font face="monospace, monospace">        return hash(self.arg)</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace">class C(object):</font></div><div><font face="monospace, monospace">    def __init__(self, arg):</font></div><div><font face="monospace, monospace">        self.arg = tuple(arg)</font></div><div><font face="monospace, monospace">        self._hash = None</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace">    def __hash__(self):</font></div><div><font face="monospace, monospace">        if self._hash is None:</font></div><div><font face="monospace, monospace">            self._hash = hash(tuple(self.arg))</font></div><div><font face="monospace, monospace">        return self._hash</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace">import timeit</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace">print(timeit.timeit('hash(f)', 'from __main__ import F; f = F(list(range(500)))'))</font></div><div><font face="monospace, monospace">print(timeit.timeit('hash(t)', 'from __main__ import T; t = T(list(range(500)))'))</font></div><div><font face="monospace, monospace">print(timeit.timeit('hash(c)', 'from __main__ import C; c = C(list(range(500)))'))</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace">results = []</font></div><div><font face="monospace, monospace">for i in range(1, 11):</font></div><div><font face="monospace, monospace">    n = i * 100</font></div><div><font face="monospace, monospace">    t1 = timeit.timeit('hash(f)', 'from __main__ import F; f = F(list(range(%d)))' % i)</font></div><div><font face="monospace, monospace">    t2 = timeit.timeit('hash(t)', 'from __main__ import T; t = T(list(range(%d)))' % i)</font></div><div><font face="monospace, monospace">    results.append(t1/t2)</font></div><div><font face="monospace, monospace">print(results)</font></div></div><div><font face="monospace, monospace"><br></font></div><div><br></div><div>F is going to create a new tuple each time and then hash it.  T already has a tuple, so we'll only pay the cost of hashing a tuple, not the cost of constructing a tuple and C caches the hash value and re-uses it once it is known.  C is the winner by a factor of 10 or more (no surprise there).  But the real interesting thing is that the the ratio of the timing results from hashing `F` vs. `T` is relatively constant in the range of my test (up to 1000 elements) and that ratio's value is approximately 1.3.  For most applications, that seems reasonable.  If you really need a speed-up, then I suppose you could recode the thing in Cython and see what happens, but I doubt that will be frequently necessary.  If you _do_ code it up in Cython, put it up on Pypi and see if people use it...</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 4, 2017 at 5:04 PM, Neil Girdhar <span dir="ltr"><<a href="mailto:mistersheik@gmail.com" target="_blank">mistersheik@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Couldn't you add __hash__ to collections.abc.Iterable ?  Essentially, expose __hash__ there; then all iterables automatically have a default hash that hashes their ordered contents.<br><br>On Wednesday, January 4, 2017 at 7:37:26 PM UTC-5, Steven D'Aprano wrote:<blockquote class="gmail_quote" style="margin:0;margin-left:0.8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">On Wed, Jan 04, 2017 at 04:38:05PM -0500, <a rel="nofollow">j...@math.brown.edu</a> wrote:

<br>> Instead of the proposals like "hash.from_iterable()", would it make sense

<br>> to allow tuple.__hash__() to accept any iterable, when called as a

<br>> classmethod? 

<br>

<br>The public API for calculating the hash of something is to call the 

<br>hash() builtin function on some object, e.g. to call tuple.__hash__ you 

<br>write hash((a, b, c)). The __hash__ dunder method is implementation, not 

interface, and normally shouldn't be called directly.

<br>

<br>Unless I'm missing something obvious, your proposal would require the 

<br>caller to call the dunder methods directly:

<br>

<br>class X:

<br>    def __hash__(self):

<br>        return tuple.__hash__(iter(self))

<br>

I consider that a poor interface design.

<br>

<br>But even if we decide to make an exception in this case, tuple.__hash__ 

<br>is currently an ordinary instance method right now. There's probably 

<br>code that relies on that fact and expects that:

<br>

<br>    tuple.__hash__((a, b, c))

<br>

<br>is currently the same as 

<br>

<br>    (a, b, c).__hash__()

<br>

<br>

<br>(Starting with the hash() builtin itself, I expect, although that is 

<br>easy enough to fix if needed.) Your proposal will break backwards 

<br>compatibility, as it requires a change in semantics:

<br>

<br>(1) (a, b, c).__hash__() must keep the current behaviour, which 

<br>means behaving like a bound instance method;

<br>

<br>(2) But tuple.__hash__ will no longer return an unbound method (actually 

<br>a function object, but the difference is unimportant) and instead will 

return something that behaves like a bound class method.

<br>

<br>Here's an implementation which does this:

<br>

<br><a href="http://code.activestate.com/recipes/577030-dualmethod-descriptor/" rel="nofollow" target="_blank">http://code.activestate.com/re<wbr>cipes/577030-dualmethod-descri<wbr>ptor/</a>

<br>

<br>so such a thing is possible. But it breaks backwards-compatability and 

<br>introduces something which I consider to be an unclean API (calling a 

<br>dunder method directly). Unless there's a *really* strong advantage to

<br>

<br>    tuple.__hash__(...)

<br>

<br>over 

<br>

<br>    hash.from_iterable(...)

<br>

(or equivalent), I would be against this change.

<br>

<br>

<br>

<br>> (And similarly with frozenset.__hash__(), so that the fast C

<br>> implementation of that algorithm could be used, rather than the slow

<br>> collections.Set._hash() implementation. Then the duplicated implementation

<br>> in _collections_abc.py's Set._hash() could be removed completely,

<br>> delegating to frozenset.__hash__() instead.)

<br>

<br>This is a good point. Until now, I've been assuming that 

<br>hash.from_iterable should consider order. But frozenset shows us that 

sometimes the hash should *not* consider order.

<br>

<br>This hints that perhaps the hash.from_iterable() should have its own 

<br>optional dunder method. Or maybe we need two functions: an ordered 

version and an unordered version.

<br>

<br>Hmmm... just tossing out a wild idea here... let's get rid of the dunder 

<br>method part of your suggestion, and add new public class methods to 

<br>tuple and frozenset:

<br>

<br>    tuple.hash_from_iter(iterable)

<br>    frozenset.hash_from_iter(itera<wbr>ble)

<br>

<br>

<br>That gets rid of all the objections about backwards compatibility, since 

<br>these are new methods. They're not dunder names, so there are no 

objections to being used as part of the public API.

<br>

<br>A possible objection is the question, is this functionality *actually* 

important enough to bother?

<br>

<br>Another possible objection: are these methods part of the sequence/set 

<br>API? If not, do they really belong on the tuple/frozenset? Maybe they 

belong elsewhere?

<br>

<br>

<br>

<br>> Would this API more cleanly communicate the algorithm being used and the

<br>> implementation, 

<br>

<br>No. If you want to communicate the algorithm being used, write some 

documentation.

<br>

<br>Seriously, the public API doesn't communicate the algorithm used for the 

<br>implementation. How can it? We can keep the same interface and change 

<br>the implementation, or change the interface and keep the implementation. 

The two are (mostly) independent.

<br>

<br>

<br>

<br>> while making a smaller increase in API surface area

> compared to introducing a new function?

<br>

<br>It's difficult to quantify "API surface area". On the one hand, we have 

<br>the addition of one or two new functions or methods. Contrast with:

<br>

<br>    * introducing a new kind of method into the built-ins (one which 

<br>      behaves like a classmethod when called from the class, and like

<br>      an instance method when called from an instance);

<br>

<br>    * changing tuple.__hash__ from an ordinary method to one of the

<br>      above special methods;

<br>

<br>    * and likewise for frozenset.__hash__;

<br>

<br>    * change __hash__ from "only used as implementation, not as 

interface" to "sometimes used as interface".

<br>

<br>

<br>To me, adding one or two new methods/functions is the smaller, or at 

least less disruptive, change.

<br>

<br>

<br>

<br>-- 

<br>Steve

<br>______________________________<wbr>_________________

<br>Python-ideas mailing list

<br></div></div><a rel="nofollow">Python...@python.org</a>

<br><span class=""><a href="https://mail.python.org/mailman/listinfo/python-ideas" rel="nofollow" target="_blank">https://mail.python.org/mailma<wbr>n/listinfo/python-ideas</a>

<br>Code of Conduct: <a href="http://python.org/psf/codeofconduct/" rel="nofollow" target="_blank">http://python.org/psf/codeofco<wbr>nduct/</a>

<br></span></blockquote></div><br>______________________________<wbr>_________________<br>

Python-ideas mailing list<br>

<a href="mailto:Python-ideas@python.org">Python-ideas@python.org</a><br>

<a href="https://mail.python.org/mailman/listinfo/python-ideas" rel="noreferrer" target="_blank">https://mail.python.org/<wbr>mailman/listinfo/python-ideas</a><br>

Code of Conduct: <a href="http://python.org/psf/codeofconduct/" rel="noreferrer" target="_blank">http://python.org/psf/<wbr>codeofconduct/</a><br></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><span><br><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:14.6667px;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;white-space:pre-wrap;background-color:transparent"><img src="https://lh6.googleusercontent.com/KjKhxPuGD6aSETVh9dazBSiS5FGrt8b37DYQ6QZAxpJIomTEygFF6ygbgOUbR0MBzycO8-LF-FspB3wTJb8LXEfIbJ6lN4H2J04EYzOZg7uaG7YcGkWesdldHuXzhLwS-etJBGO6" width="95" height="26" style="border:none" alt="pattern-sig.png"></span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:10.6667px;font-family:Arial;color:rgb(102,102,102);font-weight:700;vertical-align:baseline;white-space:pre-wrap">Matt Gilson</span><span style="font-size:10.6667px;font-family:Arial;color:rgb(102,102,102);vertical-align:baseline;white-space:pre-wrap"> // SOFTWARE ENGINEER</span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:10.6667px;font-family:Arial;color:rgb(153,153,153);vertical-align:baseline;white-space:pre-wrap">E: </span><span style="font-size:10.6667px;font-family:Arial;color:rgb(17,85,204);vertical-align:baseline;white-space:pre-wrap"><a href="mailto:matt@getpattern.com" target="_blank">matt@getpattern.com</a></span><span style="font-size:10.6667px;font-family:Arial;color:rgb(153,153,153);vertical-align:baseline;white-space:pre-wrap"> // P: 603.892.7736</span></p><br><span style="font-size:12px;font-family:Arial;color:rgb(255,0,0);vertical-align:baseline;white-space:pre-wrap;background-color:transparent">We’re looking for beta testers.  Go </span><a href="https://www.getpattern.com/meetpattern" style="text-decoration:none" target="_blank"><span style="font-size:12px;font-family:Arial;color:rgb(17,85,204);text-decoration:underline;vertical-align:baseline;white-space:pre-wrap;background-color:transparent">here</span></a><span style="font-size:12px;font-family:Arial;color:rgb(255,0,0);vertical-align:baseline;white-space:pre-wrap;background-color:transparent"> to sign up!</span></span><br></div></div>

</div>