Remove the equality operator of ExitCode
#3940
Conversation
|
Why do we need to implement our own The difference I can see to the current implementation is that tuples also compare equal to lists (and maybe other containers) with the same content. |
Codecov Report
@@ Coverage Diff @@
## develop #3940 +/- ##
===========================================
- Coverage 78.23% 70.27% -7.97%
===========================================
Files 461 461
Lines 34075 34073 -2
===========================================
- Hits 26660 23946 -2714
- Misses 7415 10127 +2712
Continue to review full report at Codecov.
|
Uhm, good question 😅 I just tried and removed the implementation and the tests I added still pass. That means it does distinguish between type, because the list with the same content does not match (which is what we want I think). So I think you are right and the fix is simply to remove it? Huh |
Right, not sure what made me believe it didn't distinguish list / tuple 😵 Indeed, simply removing it seems like the easiest fix. |
sphuber
force-pushed
the
fix/3939/serialization-exit-code
branch
from
163d0fd
to
21127ae
Compare
sphuber
changed the title
ExitCodeExitCode
| serialized = yaml.dump(exit_code) | ||
| deserialized = yaml.full_load(serialized) | ||
| assert deserialized == exit_code | ||
|
|
There was a problem hiding this comment.
Maybe we should check that the result is still an ExitCode instance here?
The operator implementation assumed that `other` would always be an instance of `ExitCode` as well and so therefore was guaranteed to contain the same attributes. However, comparing it to a bare tuple would also invoke the `ExitCode.__eq__` method and would raise and `AttributeError` as a result. The desired behavior for the equality operator is that it returns True for any other tuple of the same length and content. Since this is exactly what is implemented by the tuple base type, we do not have to override it at all.
sphuber
force-pushed
the
fix/3939/serialization-exit-code
branch
from
21127ae
to
c6ae262
Compare
We should release it soon I agree, but I think I will hold off a bit longer to see if anything else props up. If nothing is reported over the weekend I will release |
Fixes #3939
The operator implementation assumed that
otherwould always be aninstance of
ExitCodeas well and so therefore was guaranteed tocontain the same attributes. However, comparing it to a bare tuple would
also invoke the
ExitCode.__eq__method and would raise andAttributeErroras a result. The desired behavior for the equalityoperator is that it returns True for any other tuple of the same length
and content. Since this is exactly what is implemented by the tuple
base type, we do not have to override it at all.