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.

Sign up for GitHub

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

Jump to bottom

Remove old return type, mode kwarg #4503

Merged
merged 27 commits into from
Aug 24, 2023
Merged

Remove old return type, mode kwarg #4503

merged 27 commits into from
Aug 24, 2023

Conversation

rmoyard
Copy link
Contributor

@rmoyard rmoyard commented Aug 22, 2023 •
edited
Loading

Description of the Change:

This PR removes the old return type and the mode keyword argument.

Benefits:

Legacy code remove and less testing.

@github-actions
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.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.

@rmoyard rmoyard changed the title Remove old return type Remove old return type, mode kwarg and unpin numpy Aug 22, 2023
@rmoyard rmoyard changed the title Remove old return type, mode kwarg and unpin numpy Remove old return type, mode kwarg Aug 22, 2023
@rmoyard @mudit2812
Apply suggestions from code review
909705e 
Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
mudit2812
mudit2812 approved these changes Aug 23, 2023
View reviewed changes
Copy link
Contributor

@mudit2812 mudit2812 left a comment

Choose a reason for hiding this comment

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

🔥

@codecov
Copy link

codecov bot commented Aug 23, 2023 •
edited
Loading

Codecov Report

Merging #4503 (7531b2c) into master (67b92d2) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4503      +/-   ##
==========================================
- Coverage   99.67%   99.66%   -0.01%     
==========================================
  Files         378      376       -2     
  Lines       34259    32967    -1292     
==========================================
- Hits        34148    32857    -1291     
+ Misses        111      110       -1
Files Changed Coverage Δ
pennylane/__init__.py 100.00% <ø> (ø)
pennylane/gradients/finite_difference.py 100.00% <ø> (+0.53%) ⬆️
pennylane/gradients/hadamard_gradient.py 100.00% <ø> (ø)
pennylane/gradients/parameter_shift.py 100.00% <ø> (ø)
pennylane/gradients/pulse_generator_gradient.py 100.00% <ø> (ø)
pennylane/gradients/pulse_gradient.py 99.47% <ø> (-0.01%) ⬇️
pennylane/gradients/spsa_gradient.py 100.00% <ø> (ø)
pennylane/interfaces/autograd.py 100.00% <ø> (ø)
pennylane/interfaces/jax.py 99.00% <ø> (-0.04%) ⬇️
pennylane/interfaces/tensorflow_autograph.py 100.00% <ø> (ø)
... and 43 more

... and 3 files with indirect coverage changes

@rmoyard rmoyard requested a review from a team August 23, 2023 18:18
albi3ro
albi3ro approved these changes Aug 23, 2023
View reviewed changes
Copy link
Contributor

@albi3ro albi3ro left a comment

Choose a reason for hiding this comment

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

🔥 💥 !!!!

Two very minor comments, but other than that looks good :)

albi3ro
albi3ro reviewed Aug 23, 2023
View reviewed changes
Copy link
Contributor

@albi3ro albi3ro left a comment

Choose a reason for hiding this comment

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

Just realized we can also remove QuantumScript._single_measurement_shape and QuantumScript._multi_homogenous_measurement_shape.

Though we can do so in a follow up PR.

@albi3ro
Copy link
Contributor

albi3ro commented Aug 23, 2023 •
edited
Loading

Lines 241-242 and 250-254 of vjp.py might also be uncovered now. Double checking.

@timmysilv
Copy link
Contributor

can you delete the ignore-patterns=test_legacy* from tests/.pylintrc that I added? not super important, but might confuse someone many months from now

@rmoyard rmoyard enabled auto-merge (squash) August 24, 2023 14:33
@rmoyard rmoyard merged commit 6b71579 into master Aug 24, 2023
39 checks passed
@rmoyard rmoyard deleted the remove_return_type branch August 24, 2023 15:08
@albi3ro
Copy link
Contributor

albi3ro commented Aug 25, 2023

[sc-42859]

@lillian542 lillian542 mentioned this pull request Aug 29, 2023
Merged
lillian542 added a commit that referenced this pull request Aug 29, 2023
@lillian542
Update deprecations log regarding QubitDevice.statistics (#4547)
273ddc6 
The ability to pass a list of observables in `QubitDevice.statistics`
and/or use the kwarg `observables` was already removed as a side effect
of removing the old return types (see #3969 , #4503). This PR updates
the deprecations log to indicate that the deprecation cycle is complete.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@albi3ro albi3ro albi3ro approved these changes

@mudit2812 mudit2812 mudit2812 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@rmoyard @albi3ro @timmysilv @mudit2812