Closed junnaka51 closed 1 year ago
Could you resolve the conflicts?
@mitz1012 I updated the branch and resolved the conflict. Thank you for your review in advance!
@mitz1012 I have done the modifications following your review comments. Thank you for your review in advance!
- Is it necessary to modify
test_get_qubit_idx_to_qubit_val.py
?
I do not modify test_get_qubit_idx_to_qubit_val.py
in this PR. Do you mean test/assertion/get_ctrl_val/test_assert_get_ctrl_val.py ? That is indeed modified since it is necessary.
- I think we need to modify test files in
simulator/circuit_drawer
, but do you think the modification will be done in another pull request?
No it is not necessary. Please notice that quantestpy/simulator/circuit_drawer.py
is not updated in this PR.
3. It will be better to omit the word "circuit" from files names of
test_circuit_assert_ancilla_is_zero.py
,test_circuit_assert_equal_to_operator.py
,test_circuit_assert_is_zero.py
andwith_qiskit/test_circuit_assert_equal.py
, and change class names in them.
That is out of scope of this PR. That will be done elsewhere, possibly in https://github.com/QuantestPy/quantestpy/issues/175
4. Should we change the object names
test_circuit
in test files?(we can interpret it as a "test file" itself, but it originally comes from the classTestCircuit
.)
Done in https://github.com/QuantestPy/quantestpy/pull/173/commits/a6df6613f1cf205ecff4d7314255d1c9847e4f15
5. Why do two test files for converter exist in
/test/with_qiskit/
? I cannot understand the counterpart in/quantestpy/
.
In this PR I add test/with_qiskit/test_converter_to_quantestpy_circuit.py
because I added quantestpy/converter/converter_to_quantestpy_circuit.py
.
The other one test/with_qiskit/test_converter.py
has existed for a long time before this PR, and replacing/renaming this file is out of scope of this PR. If you find uncomfortable with it, please make an issue and tackle it.
- Is it necessary to modify
test_get_qubit_idx_to_qubit_val.py
?I do not modify
test_get_qubit_idx_to_qubit_val.py
in this PR. Do you mean test/assertion/get_ctrl_val/test_assert_get_ctrl_val.py ? That is indeed modified since it is necessary.
no need to modify it because _get_qubit_idx_to_qubit_val
uses PauliCircuit
.
- Should we change the object names
test_circuit
in test files?(we can interpret it as a "test file" itself, but it originally comes from the classTestCircuit
.)Done in a6df661
test_circuit
remains in this pull request, which I pointed out above.
@mitz1012 Thank you for your review. I have done the modification in https://github.com/QuantestPy/quantestpy/pull/173/commits/23730c5a43759e32352efc36761921fd5704f775
In this PR, I rename
TestCircuit
class toStateVectorCircuit
, and newly addQuantestPyCircuit
class as the base class forStateVectorCircuit
andPauliCircuit
class.It is intended that the assert methods accept an instance of
QuantestPyCircuit
and internally convert it to eitherStateVectorCircuit
orPauliCircuit
as necessary. Therefore, the two converterscvt_quantestpy_circuit_to_pauli_circuit()
andcvt_quantestpy_circuit_to_state_vector_circuit()
are also implemented:Although there are 40 files changed, 30 of them are test files, in most of which the small changes like
TestCircuit
->QuantestPyCircuit
are done. Thank you in advance!