Skip to content

Fix #324: use match for better readability and typing#434

Open
thierry-martinez wants to merge 4 commits intoTeamGraphix:masterfrom
thierry-martinez:fix/324_match
Open

Fix #324: use match for better readability and typing#434
thierry-martinez wants to merge 4 commits intoTeamGraphix:masterfrom
thierry-martinez:fix/324_match

Conversation

@thierry-martinez
Copy link
Collaborator

This commit replaces if/elif/else chains with match when the conditions perform equality comparisons on the same subject. These changes improve readibility and avoid mypy shortcomings related to type narrowing.

Before this commit, PLR1714 suggested rewriting code such as:

if cmd.kind == CommandKind.X or cmd.kind == CommandKind.Z: ...

into:

if cmd.kind in {CommandKind.X, CommandKind.Z}: ...

However, mypy does not currently infer that cmd is an instance of X or Z within the if block when set membership is used.

With this commit, the condition is expressed using an or-pattern, and type checking behaves as expected:

match cmd.kind:
    case CommandKind.X | CommandKind.Z: ...

@codecov
Copy link

codecov bot commented Feb 15, 2026

Codecov Report

❌ Patch coverage is 76.97994% with 218 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.98%. Comparing base (03a89c9) to head (1d08801).

Files with missing lines Patch % Lines
graphix/flow/exceptions.py 0.00% 68 Missing ⚠️
graphix/pattern.py 74.87% 51 Missing ⚠️
graphix/sim/tensornet.py 52.00% 24 Missing ⚠️
graphix/optimization.py 77.89% 21 Missing ⚠️
graphix/transpiler.py 93.06% 12 Missing ⚠️
graphix/qasm3_exporter.py 86.95% 9 Missing ⚠️
graphix/simulator.py 77.41% 7 Missing ⚠️
graphix/pretty_print.py 92.85% 6 Missing ⚠️
graphix/fundamentals.py 87.50% 5 Missing ⚠️
graphix/graphsim.py 73.68% 5 Missing ⚠️
... and 6 more
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@emlynsg emlynsg left a comment

Choose a reason for hiding this comment

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

I've suggested additional places to replace if else with match. Happy to discuss.
I also found a few other locations:

  • ops.py from L221
  • pauli.py from L71 (eigenstate and _repr_impl and _matmul_impl functions).

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

else:
raise ValueError(f"invalid command {cmd}")
yield f"h q{cmd.node};\n"
if cmd.angle != 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

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:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can change to match here

Copy link
Contributor

Choose a reason for hiding this comment

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

Can replace these with match too

Copy link
Contributor

Choose a reason for hiding this comment

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

Replace with match.

@thierry-martinez
Copy link
Collaborator Author

thierry-martinez commented Feb 19, 2026

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 if ...: return statements as if/elif chains.

Note that I needed to fix typing issues now that the new numba==0.64.0 is compatible with numpy==2.4. I added numba and numpy to the list of packages with pinned versions to prevent these kinds of errors from being mixed up with other PRs in the future.

Copy link
Contributor

@emlynsg emlynsg 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 to me. Would be interested in you explaining the refactoring tool at some point!

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.

2 participants

Comments