From 4d5a7eb1ccf195f674d383e2bd2d278b4175f259 Mon Sep 17 00:00:00 2001 From: Matthias Diener Date: Thu, 3 Apr 2025 16:24:07 -0500 Subject: [PATCH 1/4] add a small test for hashability of bare classes --- pytools/test/test_persistent_dict.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pytools/test/test_persistent_dict.py b/pytools/test/test_persistent_dict.py index 2cdd2a7c..edfa4c77 100644 --- a/pytools/test/test_persistent_dict.py +++ b/pytools/test/test_persistent_dict.py @@ -528,6 +528,16 @@ def update_persistent_hash(self, key_hash, key_builder): def test_class_hashing() -> None: keyb = KeyBuilder() + class WithoutUpdateMethod: + pass + + assert keyb(WithoutUpdateMethod) == keyb(WithoutUpdateMethod) + assert keyb(WithoutUpdateMethod) == "da060d601d180d4c" + + with pytest.raises(TypeError): + # does not have update_persistent_hash() = > will raise + keyb(WithoutUpdateMethod()) + class WithUpdateMethod: def update_persistent_hash(self, key_hash, key_builder): # Only called for instances of this class, not for the class itself From e35c58ab969cb341b2eec4c0928af8503a97ee27 Mon Sep 17 00:00:00 2001 From: Matthias Diener Date: Wed, 18 Jun 2025 15:54:43 -0500 Subject: [PATCH 2/4] fix local classes --- pytools/persistent_dict.py | 18 ++++++++++++-- pytools/test/test_persistent_dict.py | 36 ++++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/pytools/persistent_dict.py b/pytools/persistent_dict.py index de0b5fe4..20ad3338 100644 --- a/pytools/persistent_dict.py +++ b/pytools/persistent_dict.py @@ -244,8 +244,22 @@ def __call__(self, key: Any) -> str: # understandably don't like it. def update_for_type(self, key_hash: Hash, key: type) -> None: - key_hash.update( - f"{key.__module__}.{key.__qualname__}.{key.__name__}".encode()) + try: + module = sys.modules[key.__module__] + resolved = module + for attr in key.__qualname__.split("."): + resolved = getattr(resolved, attr) + if resolved is key: + # Globally accessible: hash based on name + self.rec(key_hash, + f"{key.__module__}.{key.__qualname__}".encode() + ) + else: + # Name collision or local class; fall back + self.rec(key_hash, f"{key.__module__}.{key.__qualname__}_{id(key)}") + except (KeyError, AttributeError): + # Can't resolve the class name, must be local + self.rec(key_hash, f"{key.__module__}.{key.__qualname__}_{id(key)}") update_for_ABCMeta = update_for_type diff --git a/pytools/test/test_persistent_dict.py b/pytools/test/test_persistent_dict.py index edfa4c77..66e807f2 100644 --- a/pytools/test/test_persistent_dict.py +++ b/pytools/test/test_persistent_dict.py @@ -525,6 +525,14 @@ def update_persistent_hash(self, key_hash, key_builder): assert keyb(MyABC3) != keyb(MyABC) != keyb(MyABC3()) +class WithoutUpdateMethodGlobal: + pass + + +class CollidingNameClass: + pass + + def test_class_hashing() -> None: keyb = KeyBuilder() @@ -532,12 +540,37 @@ class WithoutUpdateMethod: pass assert keyb(WithoutUpdateMethod) == keyb(WithoutUpdateMethod) - assert keyb(WithoutUpdateMethod) == "da060d601d180d4c" + assert keyb(WithoutUpdateMethodGlobal) == keyb(WithoutUpdateMethodGlobal) + + # This doesn't work with the function-local class "WithoutUpdateMethod", because + # local classes are instantiated at each function call, and thus their hash + # includes the id() of the class: + # assert keyb(WithoutUpdateMethod) == "N/A" + assert keyb(WithoutUpdateMethodGlobal) == "49c4673089d30507" with pytest.raises(TypeError): # does not have update_persistent_hash() = > will raise keyb(WithoutUpdateMethod()) + with pytest.raises(TypeError): + # does not have update_persistent_hash() = > will raise + keyb(WithoutUpdateMethodGlobal()) + + # {{{ test for name collisions between top-level and function-local classes + + def make_colliding_name_class(): + class CollidingNameClass: + pass + + return CollidingNameClass + + top_level_cls = CollidingNameClass + shadowed_cls = make_colliding_name_class() + + assert keyb(top_level_cls) != keyb(shadowed_cls) + + # }}} + class WithUpdateMethod: def update_persistent_hash(self, key_hash, key_builder): # Only called for instances of this class, not for the class itself @@ -558,7 +591,6 @@ class TagClass2(Tag): assert keyb(TagClass()) != keyb(TagClass2()) assert keyb(TagClass()) == "7b3e4e66503438f6" - assert keyb(TagClass2) == "690b86bbf51aad83" @tag_dataclass class TagClass3(Tag): From a722191b1176e29c742204f10e3ace6aa0f7444b Mon Sep 17 00:00:00 2001 From: Matthias Diener Date: Wed, 18 Jun 2025 16:12:14 -0500 Subject: [PATCH 3/4] bpr --- pytools/persistent_dict.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytools/persistent_dict.py b/pytools/persistent_dict.py index 20ad3338..56918c57 100644 --- a/pytools/persistent_dict.py +++ b/pytools/persistent_dict.py @@ -248,7 +248,7 @@ def update_for_type(self, key_hash: Hash, key: type) -> None: module = sys.modules[key.__module__] resolved = module for attr in key.__qualname__.split("."): - resolved = getattr(resolved, attr) + resolved = getattr(resolved, attr) # pyright: ignore[reportAny] if resolved is key: # Globally accessible: hash based on name self.rec(key_hash, From b30b29e1cf385caaeaf5d8cc52011477885df157 Mon Sep 17 00:00:00 2001 From: Matthias Diener Date: Wed, 18 Jun 2025 16:39:36 -0500 Subject: [PATCH 4/4] disable local-class hashing --- pytools/persistent_dict.py | 10 ++----- pytools/test/test_persistent_dict.py | 42 ++-------------------------- 2 files changed, 5 insertions(+), 47 deletions(-) diff --git a/pytools/persistent_dict.py b/pytools/persistent_dict.py index 56918c57..050e0be0 100644 --- a/pytools/persistent_dict.py +++ b/pytools/persistent_dict.py @@ -251,15 +251,11 @@ def update_for_type(self, key_hash: Hash, key: type) -> None: resolved = getattr(resolved, attr) # pyright: ignore[reportAny] if resolved is key: # Globally accessible: hash based on name - self.rec(key_hash, - f"{key.__module__}.{key.__qualname__}".encode() - ) + self.rec(key_hash, f"{key.__module__}.{key.__qualname__}") else: - # Name collision or local class; fall back - self.rec(key_hash, f"{key.__module__}.{key.__qualname__}_{id(key)}") + raise ValueError(f"Cannot hash function-local class '{key}'") except (KeyError, AttributeError): - # Can't resolve the class name, must be local - self.rec(key_hash, f"{key.__module__}.{key.__qualname__}_{id(key)}") + raise ValueError(f"Cannot hash function-local class '{key}'") from None update_for_ABCMeta = update_for_type diff --git a/pytools/test/test_persistent_dict.py b/pytools/test/test_persistent_dict.py index 66e807f2..e06c0978 100644 --- a/pytools/test/test_persistent_dict.py +++ b/pytools/test/test_persistent_dict.py @@ -503,8 +503,6 @@ def test_ABC_hashing() -> None: # noqa: N802 class MyABC(ABC): # noqa: B024 pass - assert keyb(MyABC) != keyb(ABC) - with pytest.raises(TypeError): keyb(MyABC()) @@ -515,62 +513,29 @@ class MyABC2(MyABC): def update_persistent_hash(self, key_hash, key_builder): key_builder.rec(key_hash, 42) - assert keyb(MyABC2) != keyb(MyABC) assert keyb(MyABC2()) class MyABC3(metaclass=ABCMeta): # noqa: B024 def update_persistent_hash(self, key_hash, key_builder): key_builder.rec(key_hash, 42) - assert keyb(MyABC3) != keyb(MyABC) != keyb(MyABC3()) + assert keyb(MyABC3()) class WithoutUpdateMethodGlobal: pass -class CollidingNameClass: - pass - - def test_class_hashing() -> None: keyb = KeyBuilder() - class WithoutUpdateMethod: - pass - - assert keyb(WithoutUpdateMethod) == keyb(WithoutUpdateMethod) assert keyb(WithoutUpdateMethodGlobal) == keyb(WithoutUpdateMethodGlobal) - - # This doesn't work with the function-local class "WithoutUpdateMethod", because - # local classes are instantiated at each function call, and thus their hash - # includes the id() of the class: - # assert keyb(WithoutUpdateMethod) == "N/A" assert keyb(WithoutUpdateMethodGlobal) == "49c4673089d30507" - with pytest.raises(TypeError): - # does not have update_persistent_hash() = > will raise - keyb(WithoutUpdateMethod()) - with pytest.raises(TypeError): # does not have update_persistent_hash() = > will raise keyb(WithoutUpdateMethodGlobal()) - # {{{ test for name collisions between top-level and function-local classes - - def make_colliding_name_class(): - class CollidingNameClass: - pass - - return CollidingNameClass - - top_level_cls = CollidingNameClass - shadowed_cls = make_colliding_name_class() - - assert keyb(top_level_cls) != keyb(shadowed_cls) - - # }}} - class WithUpdateMethod: def update_persistent_hash(self, key_hash, key_builder): # Only called for instances of this class, not for the class itself @@ -583,11 +548,8 @@ class TagClass(Tag): class TagClass2(Tag): pass - assert keyb(WithUpdateMethod) != keyb(WithUpdateMethod()) - assert keyb(TagClass) != keyb(TagClass()) - assert keyb(TagClass2) != keyb(TagClass2()) + assert keyb(WithUpdateMethod()) == keyb(WithUpdateMethod()) - assert keyb(TagClass) != keyb(TagClass2) assert keyb(TagClass()) != keyb(TagClass2()) assert keyb(TagClass()) == "7b3e4e66503438f6"