Opened 16 years ago

Closed 16 years ago

#1842 defect closed fixed (fixed)

Deprecated usage of id in __hash__ methods

Reported by: therve Owned by:
Priority: normal Milestone:
Component: core Keywords: hash id
Cc: therve, spiv Branch:
Author:

Description

I tested Python2.5 beta1 and see that id() changed to return always a positive number (and possibly a long). In http://www.python.org/download/releases/2.5/NEWS.txt you can see they warn against the use of id in a __hash__ function, which needs an int.

I count 3 occurences of this use: in web.microdom, spread.flavors, and python.usage. The last one is very harmful because it prevents me from running trial :).

Change History (6)

comment:1 Changed 16 years ago by spiv

Cc: spiv added

Without looking too closely, replacing

    def __hash__(self):
        return id(self)

with:

    def __hash__(self):
        return id(self) % sys.maxint

would probably be a simple fix (it seems happy enough with longs, so long as they fit in an int).

Perhaps Python 2.5 should do that automatically?

comment:2 Changed 16 years ago by therve

Well it's not really a fix, it's a hack. I think we should find something better: for each object, we have attributes that can be used. For example for microdom.Node, someting like this should work:

def __hash__(self):
    h = 0
    for child in self.childNodes:
        h ^= hash(child)
    return h

And for usage.Option:

def __hash__(self):
    return hash(self.shortOpt)

For spread.RemoteCache, it's less easy to choose.

comment:3 Changed 16 years ago by Jean-Paul Calderone

Owner: changed from Glyph to Jean-Paul Calderone

comment:4 Changed 16 years ago by Jean-Paul Calderone

(In [17388]) Fix hashability of three classes in Python 2.5

Refs #1842 Refs #1867

Options and RemoteCache basically just want to be hashable by identity: give them an implementation that is almost the same as the previous one, but which truncates to "short" int so that Python does not get upset. We could probably do something which exploited the negative "address space" (hashes are signed) but it seems extremely unlikely that any difference due to this would be noticable in practice.

microdom's Node class has its hash method removed entirely since it was completely superfluous: the Node class defines no custom comparison methods and so instances of Node will only compare equal to themselves, and the default hash is by identity anyway, which is what the implementation being removed was duplicated.

comment:5 Changed 16 years ago by foom

Resolution: fixed
Status: newclosed

(In [17448]) Fix almost all python 2.5 issues.

Merging branches/python-two-point-five-1867.

Authors: mostly exarkun, some jknight Reviewer: jml Closes #1867

In detail:

Fix hashability of three classes in Python 2.5: id() now returns only positive numbers, but no longer always short ints, so returning id(self) as hash breaks. Closes #1842

Truncate mtime and atime floats to be integers in conch's unix sftp server implementation. (They changed to be floats by default in python). The interface is still underspecified. Refs #1860.

Fix Newpb failure handling for new-style exceptions. Use t.p.r.qual to determine exception type names. Closes #1861

Suppress deprecation warning for raising a string exception in test_failure.FailureTestCase.testStringExceptions. Closes #1862

Refactor testPBFailures into multiple test methods to make it easier to debug. Closes #1863

Pass absolute path to interpreter as argv[0] in stdio tests. Using an absolute path lets Python's import machinery figure out what's going on. Closes #1864

Add new-style class support to jelly (absolutely necessary because exceptions are now newstyle classes). Closes #1865

Add new-style exception support to Failure. Closes #1858. Closes #1859.

Fix twisted.python.zipstream to account for the removal of file_offset from the ZipInfo class. Closes #1866.

Use reflect.qual on warning class when logging warnings.

comment:6 Changed 11 years ago by <automation>

Owner: Jean-Paul Calderone deleted
Note: See TracTickets for help on using tickets.