Skip to content
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

[unitaryHACK] Deprecated qml.inv() #1325

Merged
merged 10 commits into from
May 18, 2021
Merged

[unitaryHACK] Deprecated qml.inv() #1325

merged 10 commits into from
May 18, 2021

Conversation

tgag17
Copy link
Contributor

@tgag17 tgag17 commented May 17, 2021

Context:
Deprecated qml.inv().

Description of the Change:
Added warnings in the documentation and code

Benefits:
Want to shift users to qml.adjoint() as it is a more general function

Related GitHub Issues:
Fixes #1195

@josh146 josh146 added the unitaryhack Dedicated issue for Unitary Fund open-source hackathon label May 17, 2021
@codecov
Copy link

codecov bot commented May 17, 2021

Codecov Report

Merging #1325 (4c91803) into master (9777f09) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1325   +/-   ##
=======================================
  Coverage   98.16%   98.16%           
=======================================
  Files         148      148           
  Lines       11450    11452    +2     
=======================================
+ Hits        11240    11242    +2     
  Misses        210      210           
Impacted Files Coverage Δ
pennylane/utils.py 96.00% <100.00%> (+0.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9777f09...4c91803. Read the comment docs.

Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution @tgag17! 🎉

Just two small things before this can be merged in:

  • Don't forget to add a link/description of this change to the changelog! And to add your name to the 'Contributors' section 🙂

  • It would be good to add a test to ensure that the warning is working correctly. This test can be added to tests/test_utils.py, to the TestInv class. In particular, the

    with pytest.warns(UserWarning, match="warning message"):
        #code that raises a warning

    context manager allows easy testing of whether warnings are raised. The PyTest documentation has some more info here: https://docs.pytest.org/en/stable/warnings.html#warns

pennylane/utils.py Outdated Show resolved Hide resolved
pennylane/utils.py Outdated Show resolved Hide resolved
josh146 and others added 5 commits May 18, 2021 00:12
tests/test_utils.py Outdated Show resolved Hide resolved
tgag17 and others added 2 commits May 18, 2021 14:04
Co-authored-by: Josh Izaac <josh146@gmail.com>
@tgag17
Copy link
Contributor Author

tgag17 commented May 18, 2021

Thanks for the review and helping out with the tests @josh146! I have made the required commits

Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

Thanks for this great contribution @tgag17! This is merge ready from my end 🙂

@josh146 josh146 merged commit 6a2cf0a into PennyLaneAI:master May 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unitaryhack Dedicated issue for Unitary Fund open-source hackathon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[unitaryhack] Deprecate qml.inv
3 participants