-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add __pos__ for Parameter #12496
Add __pos__ for Parameter #12496
Conversation
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:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems sensible in spirit to me, thanks.
I guess it would have made more sense if we were able to use operator.pos
in the _apply_operation
, but that function seems to be set up to assume binary operations, so never mind on that front.
I'm not wild about introducing a floating-point number with this, though, and the same goes for __neg__
. Definitely feels like we might at least want to make them multiplication by integers rather than floats (if we can't just use operator.pos
and operator.neg
directly) to avoid introducing imprecisions into the evaluation contexts.
Please could you also add a circuits feature release note about this?
Co-authored-by: Jake Lishman <jake@binhbar.com>
Multiplication with an int seems nicer indeed. I can change it for this PR, but I am not sure whether there are any implications for |
Pull Request Test Coverage Report for Build 9358258610Details
💛 - Coveralls |
Let's make at least |
6aeea9e
to
42cc5eb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ace, thanks!
* Add __pos__ for ParameterExpression * docstring * Update qiskit/circuit/parameterexpression.py Co-authored-by: Jake Lishman <jake@binhbar.com> * Add release note * replace 1.0 by 1 * black * Reword release note --------- Co-authored-by: Jake Lishman <jake@binhbar.com> Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
* Add __pos__ for ParameterExpression * docstring * Update qiskit/circuit/parameterexpression.py Co-authored-by: Jake Lishman <jake@binhbar.com> * Add release note * replace 1.0 by 1 * black * Reword release note --------- Co-authored-by: Jake Lishman <jake@binhbar.com> Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Summary
Add
__pos__
forParameter
andParameterExpression
Details and comments
The implementation is simular to the
__neg__