Skip to content

DOC: Suggest replacement for tostring_rgb #25502

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 3 commits into from
Mar 23, 2023
Merged

Conversation

larsoner
Copy link
Contributor

@larsoner larsoner commented Mar 19, 2023

We hit this DeprecationWarning in MNE-Python (EDIT: After 9c79686) and I had to go to the commits to see the comment that argb should cover most use cases.

If this makes sense to add then I'll modify the actual warning itself, too, which would have made finding the workaround even faster.

@larsoner larsoner marked this pull request as draft March 19, 2023 11:02
@oscargus
Copy link
Member

I think this makes sense!

@tacaswell
Copy link
Member

I am 👍🏻 on this and adding the note to the message itself as well.

@anntzer
Copy link
Contributor

anntzer commented Mar 20, 2023

Note that it was also suggested to deprecate tostring_argb (#25484 (review)). In the use case of mne-python (mne-tools/mne-python#11574?) I would suggest using e.g. np.array(canvas.buffer_rgba()) instead.

@larsoner
Copy link
Contributor Author

I would suggest using e.g. np.array(canvas.buffer_rgba()) instead.

It seems like this should be the suggested replacement then, right?

@anntzer
Copy link
Contributor

anntzer commented Mar 20, 2023

It's not really a direct replacement, as the channel order is not the same; it depends on what exactly you want (but in the case of MNE python you're really just checking that the values are the same, so the channel order doesn't matter). Also this doesn't do the conversion-to-a-string that in MNE-python's case gets immediately undone by np.frombuffer.

@larsoner larsoner marked this pull request as ready for review March 20, 2023 18:23
@larsoner
Copy link
Contributor Author

Okay pushed an updated version!

@QuLogic QuLogic changed the title DOC: Suggest replacement DOC: Suggest replacement for tostring_rgb Mar 20, 2023
@QuLogic
Copy link
Member

QuLogic commented Mar 20, 2023

Since you're touching this file, can you also rename it to 25484-AL.rst?

@larsoner
Copy link
Contributor Author

@QuLogic done!

@QuLogic QuLogic added this to the v3.8.0 milestone Mar 23, 2023
@QuLogic QuLogic merged commit 2f7f62e into matplotlib:main Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants