Skip to content
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 (dagcircuit) #8252

Merged
merged 3 commits into from
Nov 19, 2022
Merged

Fix mypy errors (dagcircuit) #8252

merged 3 commits into from
Nov 19, 2022

Conversation

Randl
Copy link
Contributor

@Randl Randl commented Jun 28, 2022

Summary

Following discussion, I'm splitting #8187 by module.

Details and comments

This fixes the only mypy errors in dagcircuit as for now.

@Randl Randl requested a review from a team as a code owner June 28, 2022 13:35
@qiskit-bot
Copy link
Collaborator

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:

  • @Qiskit/terra-core

@HuangJunye HuangJunye added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Jun 28, 2022
@1ucian0 1ucian0 self-assigned this Oct 4, 2022
Copy link
Member

@1ucian0 1ucian0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for splitting it! I finally got some time to have a look at this one.

Comment on lines 103 to 107
"""Create an Instruction node"""
super().__init__()
self.op = op
self.qargs = tuple(qargs)
self.cargs = tuple(cargs)
self.qargs: Optional[Tuple[Qubit, ...]] = tuple(qargs)
self.cargs: Optional[Tuple[Clbit, ...]] = tuple(cargs)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not an expert in type annotation. Would it be better to add them into the method declaration?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, you're right; I've changed it

@Randl
Copy link
Contributor Author

Randl commented Nov 18, 2022

@1ucian0 can you approve, please? :)

@coveralls
Copy link

coveralls commented Nov 19, 2022

Pull Request Test Coverage Report for Build 3504894567

  • 7 of 7 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.04%) to 84.576%

Totals Coverage Status
Change from base Build 3503481234: -0.04%
Covered Lines: 62631
Relevant Lines: 74053

💛 - Coveralls

Copy link
Member

@1ucian0 1ucian0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your patience and your work!

@1ucian0 1ucian0 added automerge Changelog: None Do not include in changelog labels Nov 19, 2022
@mergify mergify bot merged commit 4a1a2d0 into Qiskit:main Nov 19, 2022
Cryoris pushed a commit to Cryoris/qiskit-terra that referenced this pull request Jan 12, 2023
* Fix dagcircuit mypy errors

* Move annotation to function parameter

Co-authored-by: Luciano Bello <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog Community PR PRs from contributors that are not 'members' of the Qiskit repo
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants