Add test for exceptional behavior when initializing PauliList.from_symplectic
#13751
Conversation
qiskit-bot
added
the
Community PR
|
Thank you for opening a new pull request. Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone. One or more of the following people are relevant to this code:
|
|
|
There was a problem hiding this comment.
Thanks for the increased test coverage.
As a minor comment: it's great to be detail-oriented in the PR comment, but in some cases, less can be more. It's a lot of text and images to read for a PR whose logic is obvious - it's just more work for you to write and for us to read. A PR comment like
This addresses missed testing coverage of a failure path in
PauliList.from_symplectic.
would have got across all the same information.
| try: | ||
| PauliList.from_symplectic(z, x, phase) | ||
| assert False, "Expected ValueError for 2D phase" | ||
| except ValueError as e: | ||
| assert str(e) == "phase should be at most 1D but has 2 dimensions." |
There was a problem hiding this comment.
Bare assert is rarely correct in a unittest-based test suite - in this case you want the self.assertRaisesRegex context manager.
There was a problem hiding this comment.
Refactored thanks @jakelishman. I will also keep in mind to make more concise PR description thanks for the guideline.
| @@ -221,6 +221,16 @@ def test_init_from_settings(self): | |||
| from_settings = PauliList(**pauli_list.settings) | |||
| self.assertEqual(pauli_list, from_settings) | |||
|
|
|||
| def test_from_symplectic_phase_check(self): | |||
There was a problem hiding this comment.
You'll likely need a docstring on this function to satisfy the linter (though tbh, we'd disable the rule for the test suite if it was easier to give pytest different configurations based on file path).
There was a problem hiding this comment.
Done. Thanks. Also checked locally to pass tox -elint.
ShellyGarion
added
the
mod: quantum info
Pull Request Test Coverage Report for Build 13013775255Details
💛 - Coveralls |
Summary
Improve Test Coverage for
PauliListExceptional BehaviorThis pull request addresses a testing gap in the
PauliListobject'sfrom_symplecticmethod, which was introduced in a previous pull request (#13624). The new test ensures that the exceptional behavior is thoroughly tested, enhancing the overall reliability of the Qiskit library.Details and Comments
To verify the effectiveness of this change, I have run the entire test suite locally and measured the test coverage using the following command (note that I also run the entire testing suite):
pytest --cov=$PWD/. --cov-report=xml test/python/quantum_info/operators/symplectic/test_pauli_list.pyThe results demonstrate a notable improvement in test coverage:
Coverage Comparison
Before:
After:
Please let me know if there's anything else I can provide to facilitate the review of this pull request.