Skip to content

Conversation

@acl-cqc
Copy link
Contributor

@acl-cqc acl-cqc commented Nov 26, 2025

Use instead TypeUnpacker, both to avoid duplication of logic, and to benefit from TypeUnpacker's caching.

(It does seem a little heavyweight, but there.)

rs-semver-checks seems to think this wasn't pub so we're all good?

@acl-cqc acl-cqc changed the title refactor!: Remove contain_qubits refactor: Remove contain_qubits Nov 26, 2025
@codecov
Copy link

codecov bot commented Nov 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.46%. Comparing base (7fc48c0) to head (e2bd8dd).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1283      +/-   ##
==========================================
- Coverage   79.50%   79.46%   -0.04%     
==========================================
  Files         161      160       -1     
  Lines       20642    20589      -53     
  Branches    19685    19632      -53     
==========================================
- Hits        16411    16362      -49     
+ Misses       3244     3240       -4     
  Partials      987      987              
Flag Coverage Δ
python 91.42% <ø> (ø)
qis-compiler 100.00% <ø> (ø)
rust 78.86% <100.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@acl-cqc acl-cqc requested a review from ss2165 November 26, 2025 14:54
@acl-cqc acl-cqc marked this pull request as ready for review November 26, 2025 14:54
@acl-cqc acl-cqc requested a review from a team as a code owner November 26, 2025 14:54
@ss2165
Copy link
Member

ss2165 commented Nov 26, 2025

It does seem a little heavyweight, but there

it makes sense for its original use case, we could add a wrapper around it that discards the cache per request?

@acl-cqc
Copy link
Contributor Author

acl-cqc commented Nov 26, 2025

it makes sense for its original use case, we could add a wrapper around it that discards the cache per request?

I think we are more concerned with speed than memory? In which case the cache is a good counter to the extra work in building the unneeded replacement-type-row-thing. (I had been wondering whether there'd be a good place to keep the cache and might not have gone with the refactor if there hadn't been that handy struct ModifierResolver!...that could be a wrong intuition though)

@acl-cqc acl-cqc changed the title refactor: Remove contain_qubits refactor: Remove contain_qubits, use TypeUnpacker Nov 28, 2025
@acl-cqc acl-cqc enabled auto-merge November 28, 2025 10:02
@acl-cqc acl-cqc added this pull request to the merge queue Nov 28, 2025
Merged via the queue into main with commit 3abd628 Nov 28, 2025
26 of 27 checks passed
@acl-cqc acl-cqc deleted the acl/rm_contain_qubits branch November 28, 2025 10:14
@hugrbot hugrbot mentioned this pull request Nov 27, 2025
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.

3 participants