Skip to content

Validate order kwarg in BitArray.from_bool_array() #12686

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Jul 22, 2024
Merged

Validate order kwarg in BitArray.from_bool_array() #12686

merged 7 commits into from
Jul 22, 2024

Conversation

shraddha-aangiras
Copy link
Contributor

@shraddha-aangiras shraddha-aangiras commented Jun 28, 2024 •
edited
Loading

Summary

The kwarg order in BitArray.from_bool_array() should be 'little' or 'big'. Change raises an error if order is neither of the two.
Fixes #12673

Details and comments

@shraddha-aangiras shraddha-aangiras requested review from a team as code owners June 28, 2024 13:13
@qiskit-bot qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Jun 28, 2024
@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 following people are relevant to this code:

  • @Qiskit/terra-core
  • @ajavadia
  • @levbishop
  • @t-imamichi

@CLAassistant
Copy link

CLAassistant commented Jun 28, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@t-imamichi
Copy link
Member

t-imamichi commented Jun 28, 2024 •
edited
Loading

Thank you for fixing the issue. Could you add a test to catch this error and a release note of bugfix?

@t-imamichi t-imamichi added mod: primitives Related to the Primitives module Changelog: Bugfix Include in the "Fixed" section of the changelog labels Jun 28, 2024
@shraddha-aangiras
Copy link
Contributor Author

@t-imamichi Hi, hope the test and release note added is okay. Thanks!

@t-imamichi
Copy link
Member

t-imamichi commented Jul 3, 2024 •
edited
Loading

Thank you for updating the PR. There seems style or lint errors.
Could you fix them following the instruction https://github.com/Qiskit/qiskit/blob/main/CONTRIBUTING.md#style-and-lint?
I guess applying black resolves one of the style error.

@shraddha-aangiras
Copy link
Contributor Author

@t-imamichi Sorry about the formatting issue, hope this fixed it!

@coveralls
Copy link

coveralls commented Jul 4, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9795859435

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 22 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.02%) to 89.816%
Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 4 91.86%
crates/qasm2/src/parse.rs 18 96.23%
Totals Coverage Status
Change from base Build 9794304676: -0.02%
Covered Lines: 65176
Relevant Lines: 72566
💛 - Coveralls

@t-imamichi
Copy link
Member

Thank you for fixing the style error. It looks good overall. I wrote a comment for reno.

@shraddha-aangiras @t-imamichi
Update releasenotes/notes/fix-kwarg-validation-BitArray-1bf542a1fb5c1…
376c037 
…5c6.yaml

Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>
t-imamichi
t-imamichi approved these changes Jul 5, 2024
View reviewed changes
Copy link
Member

@t-imamichi t-imamichi left a comment

Choose a reason for hiding this comment

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

LGTM

@coveralls
Copy link

coveralls commented Jul 5, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9804248280

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 15 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.006%) to 89.826%
Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 3 92.11%
crates/qasm2/src/parse.rs 12 96.69%
Totals Coverage Status
Change from base Build 9794304676: -0.006%
Covered Lines: 65183
Relevant Lines: 72566
💛 - Coveralls

@ElePT ElePT added this to the 1.2.0 milestone Jul 22, 2024
@ElePT ElePT added this pull request to the merge queue Jul 22, 2024
Merged via the queue into Qiskit:main with commit 983403b Jul 22, 2024
ElePT pushed a commit to mtreinish/qiskit-core that referenced this pull request Jul 24, 2024
@shraddha-aangiras @t-imamichi @ElePT
Validate order kwarg in BitArray.from_bool_array() (Qiskit#12686)
f879f3b 
* Validate order kwarg in BitArray

* Added releasenotes for bugfix

* Added pytest to check order

* Reformatted files

* Update releasenotes/notes/fix-kwarg-validation-BitArray-1bf542a1fb5c15c6.yaml

Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

---------

Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>
Procatv pushed a commit to Procatv/qiskit-terra-catherines that referenced this pull request Aug 1, 2024
@shraddha-aangiras @t-imamichi @Procatv
Validate order kwarg in BitArray.from_bool_array() (Qiskit#12686)
bf697a0 
* Validate order kwarg in BitArray

* Added releasenotes for bugfix

* Added pytest to check order

* Reformatted files

* Update releasenotes/notes/fix-kwarg-validation-BitArray-1bf542a1fb5c15c6.yaml

Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

---------

Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t-imamichi t-imamichi t-imamichi approved these changes

Assignees
No one assigned
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog Community PR PRs from contributors that are not 'members' of the Qiskit repo mod: primitives Related to the Primitives module
Projects
Contributor Monitoring
Status: Done
Milestone
1.2.0
Development

Successfully merging this pull request may close these issues.

validate order kwarg in BitArray.from_bool_array()
6 participants
@shraddha-aangiras @qiskit-bot @CLAassistant @t-imamichi @coveralls @ElePT