Skip to content

Catch ValueError to support pytorch (and others) plotting #26807

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 1 commit into from
Sep 18, 2023

Conversation

oscargus
Copy link
Member

@oscargus oscargus commented Sep 18, 2023

PR summary

Closes #26806

Not sure we should add it, but will probably help other libraries as well (that we do not formally support).

No way of testing on the CI as we do not want to install torch, but I promise that it works locally. 😉

PR checklist

Copy link
Member

@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

This seems fine. For patches like this, though, it would be nice to know why it broke in the first place to be sure that the raise was unintentional.

@tacaswell
Copy link
Member

This is numpy and pytorch making different choices about what to do when cast to a Python scalar

In [1]: import numpy as np

In [2]: float(np.array([[1, 2], [3, 5]]))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[2], line 1
----> 1 float(np.array([[1, 2], [3, 5]]))

TypeError: only length-1 arrays can be converted to Python scalars

@tacaswell tacaswell merged commit 3d5585a into matplotlib:main Sep 18, 2023
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Sep 18, 2023
@jklymak
Copy link
Member

jklymak commented Sep 18, 2023

This was last changed in #26303, and it indeed was made more strict there, though I think inadvertently. (Not a huge fan of micro optimizations for the reason of unintended consequences).

I don't see the harm in allowing a ValueError through, OTOH, pytorch should probably return an error consistent with numpy if they want maximum compatibility.

jklymak added a commit that referenced this pull request Sep 18, 2023
…807-on-v3.8.x

Backport PR #26807 on branch v3.8.x (Catch ValueError to support pytorch (and others) plotting)
@oscargus oscargus deleted the torchfix branch September 18, 2023 16:36
@tacaswell
Copy link
Member

OTOH, pytorch should probably return an error consistent with numpy

yeah, but then they have a changed-behavior and deprecation problem. Guess they could raise a multiply inherited exception from both TypeError and ValueError 😈

@ksunden ksunden mentioned this pull request Nov 2, 2023
5 tasks
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.

[Bug]: ValueError when plotting 2D pytorch tensor using matplotlib==3.8.0
3 participants