Skip to content

Add copy method to Options#7353

Merged
mergify[bot] merged 6 commits into
Qiskit:mainfrom
chriseclectic:options-copy
Dec 6, 2021
Merged

Add copy method to Options#7353
mergify[bot] merged 6 commits into
Qiskit:mainfrom
chriseclectic:options-copy

Conversation

@chriseclectic
Copy link
Copy Markdown
Member

Summary

Adds copy method to the Option class.

Details and comments

The option refactor changes the behaviour of copy.copy(opts) compared to the original class so that changing option values of the copy also change the original.

This adds a copy method for returning a shallow copy of option values where changes of the copy do not change the values of the original.

This copy functionality is used in qiskit-experiments.

@chriseclectic chriseclectic requested review from a team and jyu00 as code owners December 3, 2021 19:21
@coveralls
Copy link
Copy Markdown

coveralls commented Dec 3, 2021

Pull Request Test Coverage Report for Build 1544592361

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • 5 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.007%) to 83.092%

Files with Coverage Reduction New Missed Lines %
qiskit/extensions/quantum_initializer/uc.py 2 88.65%
qiskit/pulse/library/waveform.py 3 89.36%
Totals Coverage Status
Change from base Build 1537608485: -0.007%
Covered Lines: 51663
Relevant Lines: 62176

💛 - Coveralls

@levbishop
Copy link
Copy Markdown
Member

Did you consider defining __copy__() with the desired behaviour?

@chriseclectic
Copy link
Copy Markdown
Member Author

@levbishop Yes, but @jakelishman was strongly against adding that method and said to add a copy instead.

@jakelishman
Copy link
Copy Markdown
Member

I read the spec again, and I think I was wrong the first time. I think for what we are, we should implement this method as __copy__ instead. However, if we're calling __setstate__ like this to do the copy, the output type should be constructed only from its __new__, not its __init__

@mtreinish mtreinish added this to the 0.19 milestone Dec 4, 2021
Comment thread releasenotes/notes/add-backend-v2-ce84c976fb13b038.yaml Outdated
Copy link
Copy Markdown
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

LGTM

@mtreinish mtreinish added automerge Changelog: None Do not include in the GitHub Release changelog. labels Dec 6, 2021
@mergify mergify Bot merged commit efd8b13 into Qiskit:main Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changelog: None Do not include in the GitHub Release changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants