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

Add mypy static types to qbraid-qir #150

Merged
merged 6 commits into from
Aug 28, 2024
Merged

Add mypy static types to qbraid-qir #150

merged 6 commits into from
Aug 28, 2024

Conversation

TheGupta2012
Copy link
Collaborator

@TheGupta2012 TheGupta2012 commented Aug 26, 2024

Fixes #145

Summary of changes

  • Adding type checking for openqasm converter
  • Files left -
    • qbraid_qir.qasm3.visitor.py
    • qbraid_qir.cirq.*

@TheGupta2012 TheGupta2012 changed the title Add static types to openqasm Add static types to qbraid-qir Aug 26, 2024
@TheGupta2012 TheGupta2012 changed the title Add static types to qbraid-qir Add mypy static types to qbraid-qir Aug 26, 2024
Copy link

codecov bot commented Aug 26, 2024

Codecov Report

Attention: Patch coverage is 97.36842% with 5 lines in your changes missing coverage. Please review.

Files Patch % Lines
qbraid_qir/qasm3/transformer.py 88.23% 2 Missing ⚠️
qbraid_qir/cirq/convert.py 66.66% 1 Missing ⚠️
qbraid_qir/qasm3/elements.py 87.50% 1 Missing ⚠️
qbraid_qir/qasm3/visitor.py 99.07% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Member

@ryanhill1 ryanhill1 left a comment

Choose a reason for hiding this comment

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

Updates look good so far!

@TheGupta2012
Copy link
Collaborator Author

Also simplified and extended the logic of _visit_measurement method

@TheGupta2012 TheGupta2012 marked this pull request as ready for review August 27, 2024 09:25
Copy link
Member

@ryanhill1 ryanhill1 left a comment

Choose a reason for hiding this comment

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

Looks great! I noticed you used assert statements in a few places, which is a great way to assist the static type checker. However, just as a reminder, we should reserve assert for situations that are not expected to occur during normal operation—essentially, scenarios where it would be impossible to construct a program input that would cause the assert to fail. For conditions that could feasibly occur during normal operations, using exceptions like ValueError would be more appropriate. That said, it seems like the assert statements you've used are correctly placed for their intended purpose, so no changes are needed. Great job!

@TheGupta2012 TheGupta2012 merged commit 1a983e0 into main Aug 28, 2024
8 checks passed
@ryanhill1 ryanhill1 deleted the mypy-fixes branch October 18, 2024 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add static type checking with mypy
2 participants