Let _pcolorargs check C for consistency with X and Y; closes #1688 #2063

Merged
merged 3 commits into from May 28, 2013

Conversation

Projects
None yet
3 participants
Owner

efiring commented May 24, 2013

This is a minimal change to close #1688.

Owner

mdboom commented May 24, 2013

Looks good. Understanding that _pcolorargs is a private method, it still might be nice to document the purpose of the allmatch kwarg in a comment.

efiring added some commits May 24, 2013

Fix error in test_pre_transform_plotting.
This error was revealed by the new checking in _pcolorargs.
Owner

efiring commented May 25, 2013

There is an unrelated Travis test failure on Python 3.3. I think this is ready to merge.

@@ -7339,6 +7361,15 @@ def _pcolorargs(self, funcname, *args):
raise TypeError(
'Incompatible X, Y inputs to %s; see help(%s)' % (
funcname, funcname))
+ if allmatch:
+ if not (Nx == numCols and Ny == numRows):
+ raise TypeError('Dimensions of C are incompatible with'
@pelson

pelson May 28, 2013

Member

These exceptions are probably going to be quite common. I wonder if it is worth making them slightly better by providing some shape information?

I see that they are TypeErrors for consistency, but maybe they should be ValueErrors instead...

@efiring

efiring May 28, 2013

Owner

@pelson, "yes" to both of these comments. I considered the change to ValueError, but decided that was best left for a later PR, first because it could break user code, and second because I think this is not the only place where that switch should be made.

@@ -662,6 +663,20 @@ def test_pcolormesh():
ax = fig.add_subplot(133)
ax.pcolormesh(Qx,Qz,Z, shading="gouraud")
+def test_pcolorargs():
@pelson

pelson May 28, 2013

Member

Might be time to start a test_pseudo_color module. Not in this PR though.

+
+ ax.pcolormesh(np.linspace(0, 4, 7),
+ np.linspace(5.5, 8, 9),
+ np.arange(48).reshape(8, 6),
@pelson

pelson May 28, 2013

Member

This kind of thing could be quite a common problem. doh on my behalf 😉

Member

pelson commented May 28, 2013

👍 from me.

@mdboom mdboom merged commit 9d5e9ee into matplotlib:master May 28, 2013

1 check failed

default The Travis CI build failed
Details
Owner

mdboom commented May 28, 2013

I have merged this manually with the additional information in the error message as suggested by @pelson.

Owner

mdboom commented May 28, 2013

FYI: Merge commit is 7e49a5e

@efiring efiring deleted the efiring:pcolorargs branch May 28, 2013

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