Fix #324: use match for better readability and typing#434
Fix #324: use match for better readability and typing#434thierry-martinez wants to merge 4 commits intoTeamGraphix:masterfrom
match for better readability and typing#434Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #434 +/- ##
==========================================
- Coverage 88.76% 87.98% -0.78%
==========================================
Files 44 44
Lines 6308 6443 +135
==========================================
+ Hits 5599 5669 +70
- Misses 709 774 +65 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
emlynsg
left a comment
There was a problem hiding this comment.
I've suggested additional places to replace if else with match. Happy to discuss.
I also found a few other locations:
ops.pyfrom L221pauli.pyfrom L71 (eigenstateand_repr_impland_matmul_implfunctions).
graphix/transpiler.py
Outdated
There was a problem hiding this comment.
Minor but slight readability improvement:
match instr.axis:
case Axis.X:
circuit.h(instr.target)
circuit.m(instr.target, Axis.Z)
case Axis.Y:
circuit.rx(instr.target, ANGLE_PI / 2)
circuit.m(instr.target, Axis.Z)
case _:
circuit.add(instr)
Could possibly do the same to the outer if else.
graphix/qasm3_exporter.py
Outdated
| else: | ||
| raise ValueError(f"invalid command {cmd}") | ||
| yield f"h q{cmd.node};\n" | ||
| if cmd.angle != 0: |
There was a problem hiding this comment.
Similar here:
match cmd.plane:
case Plane.XY:
gate = "rx"
angle = -cmd.angle
case Plane.XZ:
gate = "ry"
angle = -cmd.angle
case Plane.YZ:
gate = "rx"
angle = cmd.angle
case _:
assert_never(cmd.plane)
| non_pauli_list.append( | ||
| command.M(node=cmd.node, angle=cmd.angle, plane=cmd.plane, s_domain=s_domain, t_domain=t_domain) | ||
| ) | ||
| else: |
graphix/pattern.py
Outdated
There was a problem hiding this comment.
Can replace these with match too
graphix/pattern.py
Outdated
146825b to
e2aa280
Compare
|
Thank you, @emlynsg, for spotting these missing cases. I updated the refactoring script, and the rewriting should now be more systematic: https://github.com/thierry-martinez/graphix-refactoring/blob/main/src/if_to_match.py The main change was to treat sequences of Note that I needed to fix typing issues now that the new |
emlynsg
left a comment
There was a problem hiding this comment.
Looks great to me. Would be interested in you explaining the refactoring tool at some point!
This commit replaces
if/elif/elsechains withmatchwhen the conditions perform equality comparisons on the same subject. These changes improve readibility and avoidmypyshortcomings related to type narrowing.Before this commit, PLR1714 suggested rewriting code such as:
into:
However,
mypydoes not currently infer thatcmdis an instance ofXorZwithin theifblock when set membership is used.With this commit, the condition is expressed using an or-pattern, and type checking behaves as expected: