diff --git a/datasketch/lsh.py b/datasketch/lsh.py index bdfff167..64e1360d 100644 --- a/datasketch/lsh.py +++ b/datasketch/lsh.py @@ -239,8 +239,10 @@ def merge(self, other: MinHashLSH, check_overlap: bool = False): of both. Note: - Only num_perm, number of bands and sizes of each band is checked for equivalency of two MinHashLSH indexes. - Other initialization parameters threshold, weights, storage_config, prepickle and hash_func are not checked. + The indexes must use the same number and size of bands, key + serialization mode, band hash function, and byte-key requirements. + Threshold, weights, and storage backend may differ when those + effective index parameters are equivalent. Args: other (MinHashLSH): The other MinHashLSH. @@ -353,10 +355,18 @@ def _insert( def __equivalent(self, other: MinHashLSH) -> bool: """Returns: - bool: If the two MinHashLSH have equal num_perm, number of bands, size of each band then two are equivalent. + bool: Whether stored keys and band hashes can be copied safely. """ - return type(self) is type(other) and self.h == other.h and self.b == other.b and self.r == other.r + return ( + type(self) is type(other) + and self.h == other.h + and self.b == other.b + and self.r == other.r + and self.prepickle == other.prepickle + and self.hashfunc is other.hashfunc + and self._require_bytes_keys == other._require_bytes_keys + ) def _merge(self, other: MinHashLSH, check_overlap: bool = False, buffer: bool = False) -> None: if self.__equivalent(other): diff --git a/test/test_lsh.py b/test/test_lsh.py index b33ec899..a9f238ec 100644 --- a/test/test_lsh.py +++ b/test/test_lsh.py @@ -17,6 +17,14 @@ def fake_redis(**kwargs): return redis +def first_band_hash(data): + return data + + +def second_band_hash(data): + return data[::-1] + + class TestMinHashLSH(unittest.TestCase): def test_init(self): lsh = MinHashLSH(threshold=0.8) @@ -342,6 +350,28 @@ def test_merge(self): lsh1.merge(lsh4, check_overlap=False) + def test_merge_rejects_different_band_hash_functions(self): + minhash = MinHash(16) + minhash.update(b"value") + destination = MinHashLSH(threshold=0.5, num_perm=16, hashfunc=first_band_hash) + source = MinHashLSH(threshold=0.5, num_perm=16, hashfunc=second_band_hash) + source.insert("source", minhash) + + with self.assertRaisesRegex(ValueError, "different initialization parameters"): + destination.merge(source) + self.assertNotIn("source", destination) + + def test_merge_rejects_different_key_serialization(self): + minhash = MinHash(16) + minhash.update(b"value") + destination = MinHashLSH(threshold=0.5, num_perm=16, prepickle=False) + source = MinHashLSH(threshold=0.5, num_perm=16, prepickle=True) + source.insert("source", minhash) + + with self.assertRaisesRegex(ValueError, "different initialization parameters"): + destination.merge(source) + self.assertNotIn("source", destination) + def test_merge_redis(self): with patch("redis.Redis", fake_redis): lsh1 = MinHashLSH(