Skip to content
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

feat: add QubitPauliOperator.get_dict #1755

Merged
merged 9 commits into from
Jan 27, 2025
Merged

feat: add QubitPauliOperator.get_dict #1755

merged 9 commits into from
Jan 27, 2025

Conversation

CalMacCQ
Copy link
Contributor

@CalMacCQ CalMacCQ commented Jan 24, 2025

Description

Please summarise the changes.

Minor change adding a get_dict export for QubitPauliOperator. This avoids having to used the undocumented private _dict attribute`

Using .get_dict as opposed to to_dict here as this new method is not intended for json serialisation.

Related issues

Please mention any github issues addressed by this PR.

Checklist

  • I have performed a self-review of my code.
  • I have commented hard-to-understand parts of my code.
  • I have made corresponding changes to the public API documentation.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have updated the changelog with any user-facing changes.

@CalMacCQ CalMacCQ requested a review from cqc-alec January 24, 2025 17:23
qps1 = QubitPauliString(Qubit(0), Pauli.Y)
qps2 = QubitPauliString(Qubit(0), Pauli.X)
op = QubitPauliOperator({qps1: 1j, qps2: 0.5})
assert op.get_dict() == op._dict
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is maybe a bit cheeky. I could do the explcit comparision if prefferred.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes I think we should do an explicit comparison, to be safe against future surprises.

qps1 = QubitPauliString(Qubit(0), Pauli.Y)
qps2 = QubitPauliString(Qubit(0), Pauli.X)
op = QubitPauliOperator({qps1: 1j, qps2: 0.5})
assert op.get_dict() == op._dict
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes I think we should do an explicit comparison, to be safe against future surprises.

pytket/pytket/utils/operators.py Outdated Show resolved Hide resolved
@CalMacCQ CalMacCQ requested review from cqc-alec and removed request for cqc-alec January 27, 2025 10:58
@@ -22,7 +22,7 @@
from hypothesis import HealthCheck, given, settings, strategies
from hypothesis.strategies._internal import SearchStrategy
from simulator import TketSimBackend, TketSimShotBackend # type: ignore
from sympy import symbols
from sympy import I, symbols
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed that pylint doesn't seem to detect the unused imports here

Dict, List, Tuple are not used.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, this is because we disable https://docs.astral.sh/ruff/rules/unused-import/ (F401) in the ruff.toml file. I disabled it because I wanted to allow unused imports in init files, but it would be good to disallow them in other files. Will make an issue to fix this.

@CalMacCQ CalMacCQ requested a review from cqc-alec January 27, 2025 11:50
@@ -22,7 +22,7 @@
from hypothesis import HealthCheck, given, settings, strategies
from hypothesis.strategies._internal import SearchStrategy
from simulator import TketSimBackend, TketSimShotBackend # type: ignore
from sympy import symbols
from sympy import I, symbols
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, this is because we disable https://docs.astral.sh/ruff/rules/unused-import/ (F401) in the ruff.toml file. I disabled it because I wanted to allow unused imports in init files, but it would be good to disallow them in other files. Will make an issue to fix this.

@CalMacCQ CalMacCQ merged commit 995cff7 into main Jan 27, 2025
30 checks passed
@CalMacCQ CalMacCQ deleted the cm/qpo_dict branch January 27, 2025 13:00
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