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

Wrong/not useful error message when plotting incompatible x and y #20452

Closed
juergspaak opened this issue Jun 16, 2021 · 10 comments
Closed

Wrong/not useful error message when plotting incompatible x and y #20452

juergspaak opened this issue Jun 16, 2021 · 10 comments
Labels
API: argument checking Good first issue Open a pull request against these issues if there are no active ones!
Milestone

Comments

@juergspaak
Copy link

juergspaak commented Jun 16, 2021

When plotting plt.plot(np.ones(10), np.ones((10,0)) it raises a ZeroDivisionError, which confused me much.

Code for reproduction

import matplotlib.pyplot as plt
import numpy as np

plt.plot(np.ones(10), np.ones((10,0)))

This raises the error:

ZeroDivisionError: integer division or modulo by zero

Expected outcome

I think however, it should either raise a ValueError, similar to plotting plt.plot(np.ones(2), np.ones(10)), i.e. something of the form:
ValueError: x and y must have same first dimension, but have shapes (10,) and (10, 0)

An alternative would be not ploting anything (similar to the behaviour of plt.plot(np.nan, np.nan) ) and potentially raise a warning.

Matplotlib version
Matplotlib version 3.3.2
module://ipykernel.pylab.backend_inline

Python version 3.8.5

[@tacaswell edited to add markup]

@jklymak jklymak added this to the v3.5.0 milestone Jun 16, 2021
@tacaswell
Copy link
Member

Picking out a bit more of the traceback we have

~/source/p/matplotlib/matplotlib/lib/matplotlib/axes/_base.py in <genexpr>(.0)
    526 else:
    527     labels = [label] * n_datasets
--> 529 result = (make_artist(x[:, j % ncx], y[:, j % ncy], kw,
    530                       {**kwargs, 'label': label})
    531           for j, label in enumerate(labels))
    533 if return_kwargs:
    534     return list(result)

ZeroDivisionError: integer division or modulo by zero

and the error is coming from 0%0. We need to check if ncx or ncy is 0 someplace above this, but it is not clear to me if it is better to raise on plot nothing. I think plotting nothing is more consistent with the way the rest of the broadcasting works?

@jklymak
Copy link
Member

jklymak commented Jun 16, 2021

I don't know

x = np.ones(10)
y = np.ones((10,0))
x+y

gives: ValueError: operands could not be broadcast together with shapes (10,) (10,0)

@tacaswell
Copy link
Member

However,

In [5]: np.ones(10) + np.ones((0, 10))
Out[5]: array([], shape=(0, 10), dtype=float64)

and I think that our broadcasting in plot follows the MATLAB column-major rules.

@jklymak
Copy link
Member

jklymak commented Jun 16, 2021

x = np.ones(10)
y = np.ones((0,10))
fig, ax = plt.subplots()
ax.plot(x,y)

Gives
ValueError: x and y must have same first dimension, but have shapes (10,) and (0, 10)

(as does any other value that is mismatched, rather than just 0)

@tacaswell
Copy link
Member

Sorry for not being clear. What I meant is that in the dimension that numpy does its broadcasting, if you run it to 0 you get back a dimension with 0 on a side. However for MATLAB reasons, the rules on how we broadcast between the x and y values for multiple datasets is a bit different than the numpy broadcasting progression

ax.plot(np.arange(10), np.ones((10, 3)))  # 3 lines
ax.plot(np.arange(10), np.ones((10, 2)))  # 2 lines
ax.plot(np.arange(10), np.ones((10, 1)))  # 1 lines
# ax.plot(np.arange(10), np.ones((10, 0)))  # 0 lines or expolde?

In this progression "do nothing" seems like the right choice.

@jklymak
Copy link
Member

jklymak commented Jun 16, 2021

Thanks, I guess given that ax.plot([]) doesn't crash, I agree that ax.plot(np.arange(10), np.ones((10, 0))) should not crash as well.

@tacaswell tacaswell added the Good first issue Open a pull request against these issues if there are no active ones! label Jun 17, 2021
@tacaswell
Copy link
Member

Labeling as a good first issue as I think it needs a short-circuit return just above the traceback identified above (or maybe where we call that generator from) and a test. It will require reading and understanding some intermediate level Python, but no API design.

@jperezg-st
Copy link

Hello, if its okay can I give it a try?

@tacaswell
Copy link
Member

@jperezg-st Please do!

@QuLogic
Copy link
Member

QuLogic commented Jun 29, 2021

Fixed by #20530.

@QuLogic QuLogic closed this as completed Jun 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API: argument checking Good first issue Open a pull request against these issues if there are no active ones!
Projects
None yet
Development

No branches or pull requests

5 participants