-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feature/pauli string test #10
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.
A couple of things here:
- The directory names
approvals
andbindings
are a bit ambiguous. I've spent the past few minutes pondering how to break up these different classes of bindings and can't come up with anything I'm excited about. The best I can come up with iscpp
,py
, andpy_cpp
, where those directories test the cpp code for correctness, the python code for correctness, and consistency between the cpp and python versions. Even as I write it, that sounds like a lot of duplicate code, but let's talk offline about a way to reuse as much code as possible. - There are some tests with three or four analogous tests with different hard coded numbers, I think we can reduce the LOC in those tests by using
pytest.mark.parameterize
.
Also let's update the CI to run all these tests (through |
Co-authored-by: James E T Smith <[email protected]>
Co-authored-by: James E T Smith <[email protected]>
I agree on a lot of points from your comment. I'll do a pass over tests and try to parametrize quick&easy cases. Also see my comment highlighting similar concerns. |
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.
PR with test coverage around python bindings