Repository navigation
Fix TypeError when checking unhashable gates in Gateset.__contains__ - #7963
pavoljuhas merged 5 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7963 +/- ##
==========================================
- Coverage 99.63% 99.63% -0.01%
==========================================
Files 1110 1110
Lines 99662 99678 +16
==========================================
+ Hits 99296 99311 +15
- Misses 366 367 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| try: | ||
| if g in self._instance_gate_families: |
There was a problem hiding this comment.
The try-except block may mask TypeError-s unrelated to g being hashable.
Please replace with if isinstance(g, Hashable) and g in ....
08a8981 to
9dd93ea
Compare
There was a problem hiding this comment.
Please add test for the failing example in #7941 to convert_to_neutral_atom_gates_test.py.
Also, see more suggestions inline.
| gate = cirq.UniformSuperpositionGate(m_value=3, num_qubits=2) | ||
| gs = cirq.Gateset(cirq.X, cirq.Y, cirq.Z) | ||
|
|
||
| # Should return False without raising TypeError |
There was a problem hiding this comment.
Nit - please avoid unnecessary comments which are apparent from the code. These are likely to go out of date and become confusing later.
| def test_gateset_contains_unhashable_gate() -> None: | ||
| """Tests that unhashable gates can be checked for containment without raising TypeError.""" | ||
| # UniformSuperpositionGate is unhashable (has __eq__ but no __hash__) | ||
| gate = cirq.UniformSuperpositionGate(m_value=3, num_qubits=2) |
There was a problem hiding this comment.
This would not cover the changed code if we add hash support to UniformSuperpositionGate later. Please use a local test-only gate class (see raw_types_test.py for such pattern), for example,
class NonHashableGate(cirq.Gate):
def _num_qubits_(self):
return 1
__hash__ = None
def __eq__(self, other):
return type(other) is NonHashableGate|
|
||
| # Should return False without raising TypeError | ||
| assert gate not in gs | ||
| assert gate.on(*cirq.LineQubit.range(2)) not in gs |
There was a problem hiding this comment.
Also add a positive contains-test for type-created cirq.Gateset(NonHashableGate).
(instance created Gateset(NonHashableGate()) fails due to unhashable object, but supporting that is out-of-scope.)
pavoljuhas
left a comment
pavoljuhas
left a comment
There was a problem hiding this comment.
LGTM with a couple of tweaks. Thank you for taking this on!
Fix
TypeError: unhashable typewhen checking if an unhashable gate is contained in aGatesetFixes #7941