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

Change DeprecationWarning and PendingDeprecationWarning to UserWarning #1211

Merged
merged 12 commits into from
Apr 30, 2021

Conversation

mariaschuld
Copy link
Contributor

@antalszava noted that the deprecation warning does not show in common editors (despite begin raised and tested). I realised that other warnings in PL have stack level 2, so adapting it here.

@github-actions
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit .github/CHANGELOG.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@codecov
Copy link

codecov bot commented Apr 14, 2021

Codecov Report

Merging #1211 (dfb0357) into master (beaf0a6) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1211   +/-   ##
=======================================
  Coverage   98.11%   98.11%           
=======================================
  Files         146      146           
  Lines       11056    11056           
=======================================
  Hits        10848    10848           
  Misses        208      208           
Impacted Files Coverage Δ
pennylane/_qubit_device.py 98.50% <ø> (ø)
pennylane/devices/default_qubit_jax.py 94.36% <ø> (ø)
pennylane/qnode.py 98.67% <ø> (ø)
pennylane/templates/embeddings/amplitude.py 100.00% <ø> (ø)
pennylane/vqe/vqe.py 92.81% <ø> (ø)

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 beaf0a6...dfb0357. Read the comment docs.

Copy link
Contributor

@antalszava antalszava left a comment

Choose a reason for hiding this comment

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

Leaving a note here too. To get the warning show locally, I either had to:

  1. add the warnings.simplefilter('always', DeprecationWarning) line so that deprecation warnings are raised or
  2. change the type of the warning from DeprecationWarning.

Option 1. would impact the entire codebase, so not entirely sure about the other deprecation warnings we have in place 🤔 Are they suppressed by default?

@josh146
Copy link
Member

josh146 commented Apr 15, 2021

What happens if we change all the DeprecationWarning warnings to PendingDeprecationWarning? Does that avoid the filtering?

@antalszava
Copy link
Contributor

Unfortunately PendingDeprecationWarning doesn't work either :(

@josh146
Copy link
Member

josh146 commented Apr 29, 2021

@mariaschuld @antalszava what is our next steps here? Is it to add our own warning filter system to PL, like other common packages do? It would be great to get this out ASAP since it is stopping users from seeing the deprecation warnings.

@josh146 josh146 added this to the v0.16.0 milestone Apr 29, 2021
@antalszava
Copy link
Contributor

@josh146 saw this just recently, I'll pick this up today!

@antalszava antalszava requested a review from josh146 April 30, 2021 17:12
@antalszava antalszava changed the title Increase stacklevel in shots DeprecationWarning Change DeprecationWarning and PendingDeprecationWarning to UserWarning Apr 30, 2021
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 helping with this @antalszava! Were these all the current deprecation warnings in the codebase?

@antalszava
Copy link
Contributor

Yep, was using sed to do a search and replace. Thought that it's sensible to change everywhere, though lmk if that's not something we'd like to go for.

@antalszava antalszava merged commit 05e50a9 into master Apr 30, 2021
@antalszava antalszava deleted the improve_shots_deprecation_warning branch April 30, 2021 17:37
@mariaschuld
Copy link
Contributor Author

Thanks for finalising this!

@josh146 josh146 modified the milestones: v0.16.0, v0.15.1 May 3, 2021
@josh146 josh146 mentioned this pull request May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants