-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Fix mypy errors (transpiler) #8266
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:
|
I'll take this one |
Pull Request Test Coverage Report for Build 5013002168
💛 - Coveralls |
6125c62
to
b3923c4
Compare
e6b5e04
to
118debc
Compare
@1ucian0 I've updated on current master |
qiskit/transpiler/passes/scheduling/alignments/align_measures.py
Outdated
Show resolved
Hide resolved
inst: Union[str, Instruction], | ||
qubits: Union[int, List[int], Qubit, List[Qubit]], | ||
inst: str | ||
| qiskit.circuit.Instruction, # FIXME: https://github.com/sphinx-doc/sphinx/issues/4961 |
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.
Sphinx seems to do the right thing currently https://qiskit.org/documentation/stubs/qiskit.transpiler.InstructionDurations.get.html
If this change is necessary for typechecking, I would prefer to leave it as it was. It looks confusing otherwise and just move the tech debt from one place to the other.
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.
I'm not sure what is the problem, but even after reverting this particular change, the doc build fails with
2023-04-16T09:39:29.4167382Z /home/vsts/work/1/s/.tox/docs/lib/python3.9/site-packages/qiskit/transpiler/instruction_durations.py:docstring of qiskit.transpiler.instruction_durations.InstructionDurations.get:1: WARNING: more than one target found for cross-reference 'Instruction': qiskit.pulse.instructions.Instruction, qiskit.pulse.instructions.instruction.Instruction, qiskit.circuit.Instruction, qiskit.circuit.instruction.Instruction
not sure why.
# Conflicts: # qiskit/transpiler/instruction_durations.py
Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
Thanks for the patience! LGTM! |
Summary
Following discussion, I'm splitting #8187 by module.
Details and comments
There are some ignores added: part to assignment to non-existing attribute, and part to decorated properties. Both appear to be justified for me. There are also some common mistakes -- e.g., using
np.array
instead ofnp.ndarray
in annotation.The are ~90 errors left:
All error list