Improved checking logic of _check_xyz in contour.py #1471

Merged
merged 3 commits into from Nov 13, 2012

Projects

None yet

3 participants

@astrofrog

At the moment, if one calls contour with:

contour(x,y,z)

and x and y are 2-d arrays with a different shape from z, the error is

raise TypeError("Inputs x and y must be 1D or 2D.")

which caused me much confusion. I dug into the code, and I realized the logic and errors could be significantly improved, so I've done that here, and added some tests. It should now give more informative/relevant errors.

@pelson pelson and 1 other commented on an outdated diff Nov 9, 2012
lib/matplotlib/tests/test_contour.py
@@ -0,0 +1,127 @@
+import numpy as np
+from matplotlib import pyplot as plt
@pelson
pelson Nov 9, 2012

FYI: All tests which use pyplot should either be an image test, or use the cleanup decroator.

@astrofrog
astrofrog Nov 9, 2012

Ok, thanks! Added.

@astrofrog

@pelson - the tests appear to be passing now.

@dmcdougall
Matplotlib Developers member

+1

@dmcdougall dmcdougall and 1 other commented on an outdated diff Nov 12, 2012
lib/matplotlib/contour.py
+
+ nx, = x.shape
+ ny, = y.shape
+
+ if nx != Nx:
+ raise TypeError("Length of x must be number of columns in z.")
+
+ if ny != Ny:
+ raise TypeError("Length of y must be number of rows in z.")
+
+ x, y = np.meshgrid(x, y)
+
+ elif x.ndim == 2:
+
+ if x.shape != z.shape:
+ raise TypeError("Shape of x does not match that of z: found {0} instead of {1}.".format(x.shape, z.shape))
@dmcdougall
dmcdougall Nov 12, 2012

This and the TypeError below stretch to longer than 78 characters. In lieu of porting the codebase to be PEP8 compliant, could you wrap these and any other lines that are too long?

@astrofrog
astrofrog Nov 12, 2012

This should be fixed now.

@pelson
Matplotlib Developers member

@astrofrog - sadly doesn't automatically merge. would you mind rebasing please?

@astrofrog

@pelson - done! All tests seem to pass on Travis.

@dmcdougall dmcdougall merged commit 8de33d0 into matplotlib:master Nov 13, 2012

1 check passed

Details default The Travis build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment