-
Notifications
You must be signed in to change notification settings - Fork 42
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
Replace the type checking using property return_type
with direct is…
#1044
Conversation
Hello. You may have forgotten to update the changelog!
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1044 +/- ##
===========================================
- Coverage 98.10% 32.91% -65.20%
===========================================
Files 233 29 -204
Lines 39079 2634 -36445
===========================================
- Hits 38339 867 -37472
- Misses 740 1767 +1027 ☔ View full report in Codecov by Sentry. |
This reverts commit d06a8cbada45a4cb0500d3d9dd134b4b6527151e.
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.
Thanks @JerryChen97 ! Looks good, just have some quick questions before approving
@maliasadi @AmintorDusko As described in description of change, the lines affected here are all in-line equivalents, therefore they should be no-effect towards any other files. However, codecov patch has been all the time 0 inside the changes of |
@JerryChen97, you might want to re-trigger the PR CIs with an empty commit. |
@AmintorDusko I removed ci build_wheels label and retriggered tests. |
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 is great! Thanks!
### Before submitting Please complete the following checklist when submitting a PR: - [ ] All new features must include a unit test. If you've fixed a bug or added code that should be tested, add a test to the [`tests`](../tests) directory! - [ ] All new functions and code must be clearly commented and documented. If you do make documentation changes, make sure that the docs build and render correctly by running `make docs`. - [ ] Ensure that the test suite passes, by running `make test`. - [ ] Add a new entry to the `.github/CHANGELOG.md` file, summarizing the change, and including a link back to the PR. - [ ] Ensure that code is properly formatted by running `make format`. When all the above are checked, delete everything above the dashed line and fill in the pull request template. ------------------------------------------------------------------------------------------------------------ **Context:** [This PR ](#1044) added two instances of `if tape_return_type is "state"` in `_adjoint_jacobian_base.py` , that introduces python SyntaxWarning, e.g. : ``` _adjoint_jacobian_base.py:197: SyntaxWarning: "is" with a literal. Did you mean "=="? if tape_return_type is "state": ``` which is also observed in CI logs during testing (e.g. [here](https://github.com/PennyLaneAI/pennylane-lightning/actions/runs/13533320431/job/37820142204#step:14:23) ) **Description of the Change:** This PR changes `is` to `==`. **Benefits:** No more syntax warning - equivalent test [here](https://github.com/PennyLaneAI/pennylane-lightning/actions/runs/13552303713/job/37878601833?pr=1070#step:14:21) **Possible Drawbacks:** **Related GitHub Issues:** [sc-85337] --------- Co-authored-by: ringo-but-quantum <[email protected]>
Context:
In the deprecations of PL0.41, the property
return_type
of classMeasurementProcess
will be deprecated very soon PennyLaneAI/pennylane#6841. Therefore, we would like to replace the use ofreturn_type
with the equivalent directisinstance
check. For individual string info use of these return types, we replace them with in-place strings.Description of the Change:
3 files are changed with equivalents in-place:
pennylane_lightning/core/_adjoint_jacobian_base.py
_get_return_type
of classLightningBaseAdjointJacobian
calledreturn_type
once, so replaced with equivalentisinstance
_check_adjdiff_supported_measurements
was only called by another private helper_process_jacobian_tape
, therefore the two Enum type returns were replaced by strings, along with their conditional values in_process_jacobian_tape
._process_jacobian_tape
also calledreturn_type
once, therefore replace with equivalentisinstance
.pennylane_lightning/core/lightning_base.py
_check_adjdiff_supported_measurements
was identified as not used by any code inside lightning. Therefore, we deleted it directly.tests/test_measurements.py
return_type
usages were replaced withisinstance
Benefits:
Consistent with both v0.40 and v0.41 of PL
Possible Drawbacks:
Not aware of yet.
Related GitHub Issues:
[sc-71892]