Skip to content

Remove backend 3.7-deprecated API #26962

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

Merged
merged 14 commits into from
Dec 6, 2023

Conversation

issamarabi
Copy link
Contributor

@issamarabi issamarabi commented Sep 29, 2023

PR summary

This PR addresses the removal of classes that were deprecated in version 3.7. (issue #26865)

Changes:

  • Removed PsBackendHelper class which was deprecated since 3.7.
  • Removed the ps_backend_helper attribute from the __getattr__ class which referenced the now-removed PsBackendHelper.
  • Removed the ServerThread class which was also deprecated since 3.7.

PR checklist

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join us on gitter for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide

We strive to be a welcoming and open project. Please follow our Code of Conduct.

@QuLogic QuLogic added this to the v3.9.0 milestone Sep 30, 2023
@QuLogic
Copy link
Member

QuLogic commented Sep 30, 2023

This looks good, but it needs an API change note for the removals. There's also something in src/_backend_agg_wrapper.cpp that's backend-related if you want to remove that too.

@QuLogic QuLogic mentioned this pull request Sep 29, 2023
14 tasks
@issamarabi
Copy link
Contributor Author

There's also something in src/_backend_agg_wrapper.cpp that's backend-related if you want to remove that too.

I've removed the deprecated methods PyBufferRegion_to_string and PyBufferRegion_to_string_argb from src/_backend_agg_wrapper.cpp as suggested. The API change notes have been updated accordingly. Please review.

@ksunden
Copy link
Member

ksunden commented Oct 9, 2023

Doc build is failing because of the build system change, should be fixed by rebasing again onto current upstream/main.

Also the changelog note wants a newline at the end (this is what is failing pre-commit CI)

This was referenced Oct 13, 2023
@Vishnu9535
Copy link

hey can i work on this issue

@QuLogic
Copy link
Member

QuLogic commented Oct 30, 2023

This is not an open issue; it's an open pull request waiting for a reply from the original submitter, @issamarabi. There is no work to be done here unless the original submitter has disappeared (and it's not clear that enough time has passed for that to be the case).

@timhoffm
Copy link
Member

timhoffm commented Nov 2, 2023

@issamarabi please report back whether you are you planning to continue this work.

issamarabi and others added 10 commits November 2, 2023 08:30
Signed-off-by: Issam Arabi <issam@cs.toronto.edu>
Signed-off-by: Issam Arabi <issam@cs.toronto.edu>
Signed-off-by: Issam Arabi <issam@cs.toronto.edu>
Signed-off-by: Issam Arabi <issam@cs.toronto.edu>
…gion_to_string_argb

This commit removes two deprecated methods, PyBufferRegion_to_string and PyBufferRegion_to_string_argb, from src/_backend_agg_wrapper.cpp, deprecated since version 3.7.

Signed-off-by: Issam Arabi <issam@cs.toronto.edu>
… methods from 3.7

This commit updates the API change notes to reflect the removal of deprecated Python classes and C++ methods.

Signed-off-by: Issam Arabi <issam@cs.toronto.edu>
Removed `to_string` and `to_string_argb` methods from `PyBufferRegion_init_type`

Signed-off-by: Issam Arabi <issam@cs.toronto.edu>
Signed-off-by: Issam Arabi <issam@cs.toronto.edu>
Signed-off-by: Issam Arabi <issam@cs.toronto.edu>
Co-authored-by: Kyle Sunden <git@ksunden.space>
issamarabi and others added 3 commits November 2, 2023 08:30
Update doc format

Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Signed-off-by: Issam Arabi <issam@cs.toronto.edu>
@issamarabi issamarabi force-pushed the remove-backend-deprecations branch from 36bc45d to 1d45c3d Compare November 2, 2023 12:31
@issamarabi
Copy link
Contributor Author

Sorry about that!

Copy link
Member

@ksunden ksunden left a comment

Choose a reason for hiding this comment

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

Should just be the small amount of clean up in the release note to quell sphinx warnings.

If you are comfortable squashing your commits down, that would also be nice (though we can do that on merge as well).

Co-authored-by: Kyle Sunden <git@ksunden.space>
@issamarabi
Copy link
Contributor Author

I created several commits to separate distinct changes. I think this will help future coders. I also wouldn't know how to create one commit message that encompasses all the changes I made.

@rcomer
Copy link
Member

rcomer commented Nov 29, 2023

I created several commits to separate distinct changes. I think this will help future coders. I also wouldn't know how to create one commit message that encompasses all the changes I made.

I think the PR title would be fine as a commit message, though you can also create a multi-line commit message if you want to add more detail. In general, Matplotlib tries to keep commits to a minimum and, while we are not overly strict about that, it would be very unusual for this project to merge 14 separate commits from a single PR.

Copy link
Member

@QuLogic QuLogic left a comment

Choose a reason for hiding this comment

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

The code looks fine; the commits could be squashed a bit.

@ksunden ksunden merged commit f80ff65 into matplotlib:main Dec 6, 2023
@QuLogic
Copy link
Member

QuLogic commented Dec 6, 2023

Thanks @issamarabi! Congratulations on your first PR to Matplotlib 🎉 We hope to hear from you again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

[MNT]: Remove 3.7-deprecated API
6 participants