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

Enable proper iteration over Statevector #8062

Merged
merged 6 commits into from
May 16, 2022

Conversation

hristog
Copy link
Contributor

@hristog hristog commented May 13, 2022

Summary

Fixes #8039.
This is achieved by enabling proper iteration over Statevector.

Details and comments

I've opted to not add a docstring to the newly-added __iter__ method, as I believe it's quite straightforward. Please, let me know if documentation should be added there and/or elsewhere.

I am new to the project, so if you could please suggest whether a release note needs to be added in the case of this PR.

Thanks!

@hristog hristog requested review from a team and ikkoham as code owners May 13, 2022 15:14
@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

@CLAassistant
Copy link

CLAassistant commented May 13, 2022

CLA assistant check
All committers have signed the CLA.

@hristog hristog force-pushed the enable-iter-over-statevector branch from 0ca7d04 to 0f9f3c6 Compare May 13, 2022 15:21
Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

Thanks for this! The formatter's complaints are just minor - I left the command to fix them up below.

Please could you add a "bugfix" release note? You can run reno new statevector-enable-iter and edit the file it says it created to do that.

test/python/quantum_info/states/test_statevector.py Outdated Show resolved Hide resolved
qiskit/quantum_info/states/statevector.py Show resolved Hide resolved
@jakelishman jakelishman added stable backport potential The bug might be minimal and/or import enough to be port to stable Changelog: Bugfix Include in the "Fixed" section of the changelog labels May 13, 2022
@jakelishman jakelishman added this to the 0.20.2 milestone May 13, 2022
@hristog
Copy link
Contributor Author

hristog commented May 13, 2022

Thanks for this! The formatter's complaints are just minor - I left the command to fix them up below.

Please could you add a "bugfix" release note? You can run reno new statevector-enable-iter and edit the file it says it created to do that.

Thanks for reviewing the PR and for providing instructions regarding the release note and formatting checking protocols!

@coveralls
Copy link

coveralls commented May 16, 2022

Pull Request Test Coverage Report for Build 2334794780

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.001%) to 84.372%

Totals Coverage Status
Change from base Build 2334792534: 0.001%
Covered Lines: 54526
Relevant Lines: 64626

💛 - Coveralls

Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks for the changes!

@hristog
Copy link
Contributor Author

hristog commented May 16, 2022

Looks good to me, thanks for the changes!

Thank you for the constructive discussion!

@jakelishman
Copy link
Member

There's no need to manually update the PR - we've got a bot that does it for us. There's quite a few things in the queue to merge right now, which is why it looks like it's just sitting, but it will do it on its own.

@jakelishman
Copy link
Member

@Mergifyio refresh

@mergify
Copy link
Contributor

mergify bot commented May 16, 2022

refresh

✅ Pull request refreshed

@mergify mergify bot merged commit da9ae54 into Qiskit:main May 16, 2022
mergify bot pushed a commit that referenced this pull request May 16, 2022
* Enable proper iteration over Statevector

* Add Statevector.__len__ and add a release note

* Simplify release note

* Update test_statevctor_iter

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit da9ae54)
mergify bot added a commit that referenced this pull request May 16, 2022
* Enable proper iteration over Statevector

* Add Statevector.__len__ and add a release note

* Simplify release note

* Update test_statevctor_iter

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit da9ae54)

Co-authored-by: Hristo Georgiev <hristog@users.noreply.github.com>
ajavadia pushed a commit to ajavadia/qiskit that referenced this pull request May 31, 2022
* Enable proper iteration over Statevector

* Add Statevector.__len__ and add a release note

* Simplify release note

* Update test_statevctor_iter

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

violation of iterator protocol for Statevector
5 participants