refactor(api): Compare opentrons.types.Point with strict floating-point equality#18215
Merged
SyntaxColoring merged 2 commits intoedgefrom May 2, 2025
Merged
refactor(api): Compare opentrons.types.Point with strict floating-point equality#18215SyntaxColoring merged 2 commits intoedgefrom
opentrons.types.Point with strict floating-point equality#18215SyntaxColoring merged 2 commits intoedgefrom
Conversation
ddcc4
pushed a commit
that referenced
this pull request
May 16, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it.
ddcc4
pushed a commit
that referenced
this pull request
May 16, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it.
ddcc4
pushed a commit
that referenced
this pull request
May 16, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it.
ddcc4
pushed a commit
that referenced
this pull request
May 16, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it.
ddcc4
pushed a commit
that referenced
this pull request
May 16, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it.
ddcc4
pushed a commit
that referenced
this pull request
May 16, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it.
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it.
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it.
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it.
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it. (cherry picked from commit 4250aab)
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it. (cherry picked from commit 4250aab)
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it. (cherry picked from commit 4250aab)
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it. (cherry picked from commit 4250aab)
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it. (cherry picked from commit 4250aab)
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it. (cherry picked from commit 4250aab)
ddcc4
pushed a commit
that referenced
this pull request
May 19, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it. (cherry picked from commit 4250aab)
ddcc4
pushed a commit
that referenced
this pull request
May 19, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it. (cherry picked from commit 4250aab)
ddcc4
pushed a commit
that referenced
this pull request
May 19, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it. (cherry picked from commit 4250aab)
ddcc4
pushed a commit
that referenced
this pull request
May 20, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it. (cherry picked from commit 4250aab)
ddcc4
pushed a commit
that referenced
this pull request
May 20, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it. (cherry picked from commit 4250aab)
ddcc4
pushed a commit
that referenced
this pull request
May 22, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it. (cherry picked from commit 4250aab)
ddcc4
pushed a commit
that referenced
this pull request
May 23, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it. (cherry picked from commit 4250aab)
ddcc4
pushed a commit
that referenced
this pull request
May 24, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it. (cherry picked from commit 4250aab)
ddcc4
pushed a commit
that referenced
this pull request
May 24, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it. (cherry picked from commit 4250aab)
ddcc4
pushed a commit
that referenced
this pull request
May 29, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it. (cherry picked from commit 4250aab)
ddcc4
pushed a commit
that referenced
this pull request
May 29, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it. (cherry picked from commit 4250aab)
ddcc4
pushed a commit
that referenced
this pull request
May 29, 2025
…oint equality (#18215) For a long time, `opentrons.types.Point` has overridden its `==` operator to return `True` if the two points are *numerically approximately* equal to each other, instead of *actually* equal. This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests? I personally find this very surprising. If I do `a == b`, I strongly expect that to mean actual equality—if I wanted approximation, I would have used `isclose()`. In other words, I expect `point_a == point_b` to behave exactly like `(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z)`. Also, it breaks [the rule that objects that compare equal must have the same hash value](https://docs.python.org/3/reference/datamodel.html#object.__hash__). ```python >>> a = Point() >>> b = Point(z=1e-20) >>> a == b True >>> hash(a) 3010437511937009226 >>> hash(b) 9180445109486892018 ``` So this PR restores strict equality checking. The old behavior is preserved as an explicit method, `point.elementwise_isclose(other)`, for the benefit of the few tests that were implicitly relying on it. (cherry picked from commit 4250aab)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
For a long time,
opentrons.types.Pointhas overridden its==operator to returnTrueif the two points are numerically approximately equal to each other, instead of actually equal.This was introduced in commit d13ed34 of PR #5583, possibly for the benefit of some calibration unit tests?
I personally find this very surprising. If I do
a == b, I strongly expect that to mean actual equality—if I wanted approximation, I would have usedisclose(). In other words, I expectpoint_a == point_bto behave exactly like(point_a.x == point_b.x) and (point_a.y == point_b.y) and (point_a.z == point_b.z).Also, it breaks the rule that objects that compare equal must have the same hash value.
So this PR restores strict equality checking. The old behavior is preserved as an explicit method,
point.elementwise_isclose(other), for the benefit of the few tests that were implicitly relying on it.Test Plan and Hands on Testing
Risk assessment
High.
Pointis used, uh, everywhere, and it's impossible for us to audit all the call sites ofPoint.__eq__to determine if any of them were implicitly relying on the approximation.Review requests
Is this actually a good idea?