Fix mypy errors (quantum_info)#8257
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 everyone. One or more of the the following people are requested to review this:
|
That issue is closed. Do you still get an error if you run with the latest version of mypy? |
Pull Request Test Coverage Report for Build 4117876587
💛 - Coveralls |
|
The code related to this issue is removed anyway. I've rebased on current master and updated correspondingly |
kevinsung
left a comment
There was a problem hiding this comment.
Thanks! Are there any more mypy issues in this module? If so, could you report the output?
|
There are a few left: All except one are since |
|
The last one seems like it's related to one of your changes, right? Could you please post the mypy outputs both before and after the changes you made? |
|
Ok, this is a consequence of the fact that Not sure what is the appropriate fix? Maybe just adding iter function |
|
|
||
| # expose one of the basis gates so others can know what this decomposer targets | ||
| embodiment_circuit = next(iter(self.embodiments.items()), ([], []))[1] | ||
| embodiment_circuit: list | QuantumCircuit = next(iter(self.embodiments.items()), ([], []))[ |
There was a problem hiding this comment.
Shouldn't it be CircuitInstruction instead of QuantumCircuit?
There was a problem hiding this comment.
I don't think so. This docstring mentions "circuit"
https://github.com/Qiskit/qiskit-terra/blob/089017cda48fadb7b4d8e7f11a9c5d5074ec5751/qiskit/quantum_info/synthesis/xx_decompose/decomposer.py#L66-L68
and tests use "circuit"
https://github.com/Qiskit/qiskit-terra/blob/089017cda48fadb7b4d8e7f11a9c5d5074ec5751/test/python/quantum_info/xx_decompose/test_decomposer.py#L82-L97
maybe the code should accept both tho.
There was a problem hiding this comment.
Looking at the code, it seems that
next(iter(self.embodiments.items()), ([], []))should be a QuantumCircuit. But then embodiment_circuit is actually obtained by indexing into this object:
next(iter(self.embodiments.items()), ([], []))[1]There was a problem hiding this comment.
self.embodiments is a dictionary, so self.embodiments.items() contains tuples, and thus indexing will return the second element of first tuple, i.e., circuit
There was a problem hiding this comment.
Ah, ok. It seems this line can be significantly simplified by calling self.embodiments.values() instead of items. You can go ahead and make that change if you like, but it's not necessary.
kevinsung
left a comment
There was a problem hiding this comment.
Thanks! If you decide to update your other mypy PRs, please include the mypy output both before and after your changes. That would make the review easier.
|
@kevinsung Thanks! I've added the mypy output after changes, but keeping PRs up-to-date is a bit annoying, so I'm waiting for some of the maintainers to engage on specific PR before updating it. Feel free to ping me if you want me to update some specific ones. |
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Summary
Following discussion, I'm splitting #8187 by module.
Details and comments
There are 8 errors left:
All are related to the fact that gates, even though inheriting from
Gatehave different constructor signature. Not sure what the proper fix is.The ignores are related to python/mypy#1362