-
Notifications
You must be signed in to change notification settings - Fork 127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade unittest equality method #1132
Upgrade unittest equality method #1132
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a really nice upgrade and consolidation of the current comparison test code and a good use of multimethod. My comments are mainly about edge cases in the equality check.
releasenotes/notes/add-test-equality-checker-dbe5762d2b6a967f.yaml
Outdated
Show resolved
Hide resolved
releasenotes/notes/add-test-equality-checker-dbe5762d2b6a967f.yaml
Outdated
Show resolved
Hide resolved
releasenotes/notes/add-test-equality-checker-dbe5762d2b6a967f.yaml
Outdated
Show resolved
Hide resolved
releasenotes/notes/add-test-equality-checker-dbe5762d2b6a967f.yaml
Outdated
Show resolved
Hide resolved
Thanks @nkanazawa1989, the fixes look good to me. I wonder if there should be an optional |
This sounds like a good idea. Done in 47dd5d5. I also added an option for numerical precision.
Yes. We can wait for 0.24rc1. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good now, thanks for adding strict_type
. I'm good with merging once terra 0.24 is released and set as a requirement.
- Cleanup doc - Add handling for edge case Co-authored-by: Helena Zhang <[email protected]>
47dd5d5
to
f96568d
Compare
Summary
Current implementation of equality check, i.e.
QiskitExperimentsTestCase.json_equiv
, is not readable and scalable because it implements equality check logic for different types in a single method. This PR adds new test moduletest/extended_equality.py
which implements new equality check dispatcheris_equivalent
.Developers no longer need to specify
check_func
in theassertRoundTripSerializable
andassertRoundTripPickle
methods unless they define custom class for a specific unittest. This simplifies unittests and improves readability of equality check logic (and test becomes more trustable).This PR adds new software dependency in develop; multimethod
Among several similar packages, this is chosen in favor of
functools.singledispatch
typings
, e.g.Union