Implemented a type check in copy method of QuantumCircuit class #10521
Conversation
|
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 every 8000 one. One or more of the the following people are requested to review this:
|
There was a problem hiding this comment.
Thanks for looking at this.
In this case, it'd actually be a little better to put the test in QuantumCircuit.copy_empty_like, because then both QuantumCircuit.copy and copy_empty_like will both gain the new test.
Please could you also add a one-line test of each method in test/python/circuit/test_circuit_operations.py?
|
By the way, there's no need to close and open PRs if you're changing it - you can just push commits to the PR you've already got open. You only need a new PR if you're starting a separate change. |
|
By the test do you mean adding the following? |
|
Sure, something like class TestCircuitOperations(QiskitTestCase):
def test_circuit_copy_rejects_bad_types(self):
qc = ...
with self.assertRaises(TypeError):
qc.copy(name=...) |
|
Sorry, I realise I wasn't super clear: I didn't mean literally one line, I was being a bit imprecise and just meant that it only needs to be a very short test. |
|
I did take it literally. No problem :) |
|
Should I write tests for both methods under just one function or two? What I mean to say is: Should I do this - or Sorry for so many questions. Edit - right now doing it with two functions to ensure a clear documentation and testing. |
|
Don't worry it's ok - we all need to learn. Here, it's probably best for you to write two separate functions. It's ok in tests if the code is a little more duplicated, because test code should look really obviously correct to someone reading the test, as far as possible. |
|
@jakelishman Thank you |
Pull Request Test Coverage Report for Build 5749765666
💛 - Coveralls |
…it#10521) * Implemented a type check in copy method of QuantumCircuit class * Made copy method code more efficient and added a circuit operation test * Corrected and made clear documentation of copy method - QuantumCircuit * Fix wording --------- Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Summary
Forces the arguments to be of string or None type for copy method of QuantumCircuit Class hence solving issues that may rise due to unsupported/unwanted types.
Details and comments
The code checks if the 'name' argument is of string or None type.
If 'name' is of string or None type, Then the following code creates a copy of the provided circuit.
Else, It raises a TypeError.
Closes #10487