Skip to content

Fix some pyright issues #163

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

Merged
merged 16 commits into from
Apr 20, 2025
Merged

Fix some pyright issues #163

merged 16 commits into from
Apr 20, 2025

Conversation

david-pl
Copy link
Collaborator

Got it down to 14 errors (starting out with 69). Not sure if all changes make sense.

The errors that are left are mostly ones such as this:

kirin-workspace/bloqade-circuit/src/bloqade/stim/emit/stim.py:35:9 - error: Method "emit_block" overrides class "EmitABC" in an incompatible manner
    Return type mismatch: base method returns type "str", override returns type "str | None"
      Type "str | None" is not assignable to type "str"
        "None" is not assignable to "str" (reportIncompatibleMethodOverride)

I'm not sure what the best way to deal with those method overrides is.

Also, there's still a bunch of errors in visual/animation, which I could fix, but it would require some type assertions here and there (i.e. actually raising an error if the type expectation is not met) and I didn't want to go and break @kaihsin animation scripts.

We can discuss & continue here, or just merge (if the changes are fine) and continue in a separate PR, since this already improves things a bit.

@david-pl david-pl requested review from Roger-luo, weinbe58 and johnzl-777 and removed request for Roger-luo April 16, 2025 11:15
@kaihsin
Copy link
Contributor

kaihsin commented Apr 16, 2025

For the CU3 bug, would you mind also adding unit tests for one of the angle to be zero?

@david-pl
Copy link
Collaborator Author

Oh, CI fails because I added a method get_present_trait to kirin locally (just hitting F12 in vscode can do that). I'll open another PR there, because I think it makes sense to have it. But I we can also use cast here to assert types for the linter.

Copy link
Member

@weinbe58 weinbe58 left a comment

Choose a reason for hiding this comment

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

LGTM but I think we might have some issues with the implementation in some areas not just linting.

Comment on lines 205 to 212
origin_qubits = tuple(
[frame.get(input_elem).origin_qubit for input_elem in stmt.inputs]
[
frame.get_casted(input_elem, AddressWire).origin_qubit
for input_elem in stmt.inputs
]
)
new_address_wires = tuple(
[AddressWire(origin_qubit=origin_qubit) for origin_qubit in origin_qubits]
Copy link
Member

Choose a reason for hiding this comment

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

This is also wrong you should just return all the values from the frame because there is no dependency on what the value is.

return frame.get_values(stmt.inputs)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@johnzl-777 how about this one?

Copy link
Member

Choose a reason for hiding this comment

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

I talked to @johnzl-777 in person, he is OK with this change.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed it!

Roger-luo pushed a commit to QuEraComputing/kirin that referenced this pull request Apr 16, 2025
Noticed in QuEraComputing/bloqade-circuit#163
that this is useful for linting, but also just for shortening code here
and there. I also replaced code that raises a `ValueError` if the result
from `get_trait` is `None` here (2nd commit).
Roger-luo pushed a commit to QuEraComputing/kirin that referenced this pull request Apr 17, 2025
Noticed in QuEraComputing/bloqade-circuit#163
that this is useful for linting, but also just for shortening code here
and there. I also replaced code that raises a `ValueError` if the result
from `get_trait` is `None` here (2nd commit).
@Roger-luo
Copy link
Member

fyi. QuEraComputing/kirin#370 is in v0.16.7 now

@Roger-luo
Copy link
Member

test failure seems due to a simple typo

Copy link

codecov bot commented Apr 18, 2025

Copy link
Contributor

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
6202 5316 86% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
src/bloqade/analysis/address/impls.py 62% 🟢
src/bloqade/noise/native/model.py 88% 🟢
src/bloqade/qasm2/parse/lowering.py 59% 🟢
src/bloqade/qasm2/passes/fold.py 89% 🟢
src/bloqade/qasm2/rewrite/heuristic_noise.py 95% 🟢
src/bloqade/qasm2/rewrite/native_gates.py 63% 🟢
src/bloqade/qasm2/rewrite/register.py 85% 🟢
src/bloqade/qasm2/rewrite/uop_to_parallel.py 69% 🟢
src/bloqade/qbraid/schema.py 81% 🟢
src/bloqade/squin/analysis/nsites/analysis.py 84% 🟢
src/bloqade/visual/animation/base.py 50% 🟢
TOTAL 75% 🟢

updated for commit: 29da51e by action🐍

@david-pl david-pl requested a review from weinbe58 April 18, 2025 06:55
@Roger-luo
Copy link
Member

bump @weinbe58 for a review.

Copy link
Member

@weinbe58 weinbe58 left a comment

Choose a reason for hiding this comment

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

LGTM

@weinbe58 weinbe58 merged commit 1cd01ec into main Apr 20, 2025
10 of 11 checks passed
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.

5 participants