This repository has been archived by the owner on Jun 12, 2023. It is now read-only.
Avoid implict marshmallow usage #400
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The Results class in terra is no longer constructed via marshmallow since #4030 merged. When updating the counts in a result object the dictionary can now be set directly instead of needlessly wrapping it in a marshmallow container for a dictionary. Besides fixing compatibility with terra 0.15.0/master it also will improve performance because there is no extra overhead from marshmallow constructing and running validation prior to updating the attribute.
|
This had to be combined with the fixes in #401 because they're all needed to unblock CI |
mtreinish
added a commit
to mtreinish/qiskit-ignis
that referenced
this pull request
Jun 18, 2020
The change made in qiskit-community#400 (which was missed because it got integrated into the qiskit-community#401 PR which was needed to unblock CI on master) should not have been backported. It was only necessary for compat with terra master and terra 0.15.0 because the validation module is removed in 0.15.0. This is breaking measurement mitigation on the stable branch and 0.3.1 release which needs to work with terra 0.14.x. This commit reverts that change so the we're wrapping the counts in qiskit.validation.Obj and the Results object is useable in marshmallow with released terra.
chriseclectic
pushed a commit
that referenced
this pull request
Jun 18, 2020
* Revert #400/#401 from stable branch The change made in #400 (which was missed because it got integrated into the #401 PR which was needed to unblock CI on master) should not have been backported. It was only necessary for compat with terra master and terra 0.15.0 because the validation module is removed in 0.15.0. This is breaking measurement mitigation on the stable branch and 0.3.1 release which needs to work with terra 0.14.x. This commit reverts that change so the we're wrapping the counts in qiskit.validation.Obj and the Results object is useable in marshmallow with released terra.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
The Results class in terra is no longer constructed via marshmallow
since Qiskit/qiskit#4030 merged. When updating the counts in a result object the
dictionary can now be set directly instead of needlessly wrapping it in
a marshmallow container for a dictionary. Besides fixing compatibility
with terra 0.15.0/master it also will improve performance because there
is no extra overhead from marshmallow constructing and running
validation prior to updating the attribute.
Details and comments