-
Notifications
You must be signed in to change notification settings - Fork 11
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
Migrate express
functionality and AbstractRepresentation
type to QuantumInterface
#93
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Three minor things:
- The comment about
AbstractUse
-- I think it makes sense to move this too. E.g. if QuantumClifford is converting an Operator it needs to know whether the user wants a Stabilizer or a CliffordOperator - The docstring update (or move)
- Check that
express
still shows in the documentation -- now that it does not belong to QuantumSymbolics, it might need to be explicitly included.
Otherwise feel free to merge once CI passes, the Project.toml is bumped, without waiting for another review. I guess that requires the other PR in QuantumInterface to be merged first.
Marking it as a draft to remove it from my review queue for now. Convert it back when ready and merge (or ping me to merge). |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #93 +/- ##
==========================================
- Coverage 75.46% 75.22% -0.25%
==========================================
Files 19 19
Lines 803 791 -12
==========================================
- Hits 606 595 -11
+ Misses 197 196 -1 ☔ View full report in Codecov by Sentry. |
It still shows in the docs. |
#92 qojulia/QuantumInterface.jl#46
The problem with this PR and the one in QuantumInterface is that a type piracy is introduced here:
I suppose we could migrate
AbstractUse
and its subtypes to QuantumInterface as well, butwould then be a type piracy. Plus
AbstractUse
is intended for dealing with edge cases in symbolic-to-Clifford translations, so to me it doesn't make sense to migrate it up to QuantumInterface.