KeyBuilder: change int to direct str representation #261
Conversation
| assert keyb(1) == keyb("1") | ||
| assert keyb(1.0) != keyb("1.0") # '1.0' uses hex representation | ||
| assert keyb(1+1j) == keyb("(1+1j)") |
There was a problem hiding this comment.
Not sure if this is an issue. If it is, we could change the KeyBuilder to (int, str(key)).
| @@ -432,9 +432,13 @@ def test_scalar_hashing() -> None: | |||
| assert keyb(np.int32(1)) == keyb(np.int32(1)) | |||
| assert keyb(np.int32(2)) != keyb(np.int32(1)) | |||
| assert keyb(np.int64(1)) == keyb(np.int64(1)) | |||
| assert keyb(1) == keyb(np.int64(1)) | |||
| assert keyb(1) != keyb(np.int64(1)) | |||
There was a problem hiding this comment.
Wait, I thought the goal was to keep them the same?
There was a problem hiding this comment.
From your answer to my question in #260, my interpretation was that they don't need to be the same. Changing the code like in this PR perhaps also addresses #262.
There was a problem hiding this comment.
They need to be the same. The fundamental property of a hash function
>>> np.int32(1) == 1
True
our hands are tied. I'm sorry if my early comment was ambiguous.
There was a problem hiding this comment.
Hmm, we already break this principle in some (many?) places:
>>> from pytools.persistent_dict import KeyBuilder
>>> keyb = KeyBuilder()
>>> 1.0 == 1
True
>>> keyb(1.0) == keyb(1)
False(with current main branch)
matthiasdiener
force-pushed
the
keyb-int-str
branch
from
9f739d0 to
2a19397
Compare
matthiasdiener
force-pushed
the
keyb-int-str
branch
from
2a19397 to
7546ab8
Compare
|
Just FYI, the tests pass on PPC64 BE with this patch applied. |
|
Unsubscribing... @-mention or request review once it's ready for a look or needs attention. |
Followup of #260.