Skip to content

Conversation

@jgsogo
Copy link

@jgsogo jgsogo commented Oct 10, 2025

Summary

This PR is extracting to a new function the logic to parse QPY headers. It also leverages on the existing get_qpy_version to get the QPY version.

It's a pure refactor.

Note.- I'm not sure if we want to add this or not to the public/stable API, or it will be just for internal consumption. Once we agree on this we can decide if we need to add tests and/or release notes and docs.

@CLAassistant
Copy link

CLAassistant commented Oct 10, 2025

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@jakelishman
Copy link
Member

I'm not sure if we want to add this or not to the public/stable API, or it will be just for internal consumption.

If this is only being done as an internal refactor for Qiskit itself, I'm not sure it's necessary - just feels like churn if there's no underlying cause. If by "internal" you mean an IBM package other than Qiskit, then there is no such thing as privileged access to Qiskit internals, and you would need this to be public interface, and it would help to have a motivating use case in a feature request.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 18403049920

Details

  • 16 of 17 (94.12%) changed or added relevant lines in 1 file are covered.
  • 6 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.02%) to 88.262%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/qpy/interface.py 16 17 94.12%
Files with Coverage Reduction New Missed Lines %
crates/circuit/src/parameter/parameter_expression.rs 1 81.99%
crates/qasm2/src/lex.rs 5 92.03%
Totals Coverage Status
Change from base Build 18391394072: 0.02%
Covered Lines: 93201
Relevant Lines: 105596

💛 - Coveralls

@jgsogo
Copy link
Author

jgsogo commented Oct 10, 2025

Thanks a lot for your quick turnaround 💌 .

We are using the logic to parse the QPY headers in the backend. I would love if this can be part of the public interface. If not, we should think if we prefer to rely on a private function (no stability commitment) or keep maintaining a copy of this "cryptic" logic on our side.

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.

4 participants