Guess the complementary dimension when only one is passed to pcolormesh #1291

vnoel · 2017-03-02T13:31:16Z

closes guess the second coordinate when only one is passed to pcolormesh() #1290

fmaussion · 2017-03-02T14:25:07Z

Seems pretty safe to me, thanks! Could you add some tests, too? (at least for proper code coverage)

A good place to put a test would be here:
https://github.com/pydata/xarray/blob/master/xarray/tests/test_plot.py#L638

(I suspect your PR brakes these tests, btw)

fmaussion · 2017-03-02T14:25:25Z

Does anyone know why Travis doesn't start?

vnoel · 2017-03-02T15:22:03Z

I have added a couple of exceptions to the _infer_xy_labels function and updated the tests as best I could

fmaussion · 2017-03-02T15:41:44Z

Thanks! We'll have to wait a little bit here, as for some reason the travis tests do not run

vnoel · 2017-03-02T15:59:49Z

thanks ! Do we need to worry about the "appveyor" build fails? The failure seems to happen somewhere unrelated

vnoel · 2017-03-03T11:25:06Z

Can anyone knowledgeable tell me how to fix the appveyor build? It's complaining of a failing test_dask_distributed_integration_test...

shoyer · 2017-03-03T23:13:24Z

I think I managed to restart the Travis-CI integration. It was disable for some mysterious reason. Maybe try pushing again or rebasing to see if that triggers them again.

See #1292 for the failing dask distibuted tests. Those can be safely ignored here.

shoyer

also needs a note in "whats-new.rst"

shoyer · 2017-03-03T23:14:37Z

xarray/plot/utils.py

-        raise ValueError('cannot supply only one of x and y')
+    elif x is None:
+        if y not in darray.dims:
+            raise ValueError('y must be a coordinate variable')


This is a little misleading -- I would say "y must be a coordinate variable if x is not supplied"

I'm not sure I understand the logic. The user requests a y variable that is not in the coordinates. It would not work even if she supplied a valid x coordinate. Your messages suggests it would.

Maybe "y is not a coordinate variable" is more to the point (and the same for x)?

Mh, that's actually a good point. x and y can also be optional (e.g. 2D) coordinates. What about:

y must be a dimension coordinate if x is not supplied

?

@fmaussion : You are proposing the same message as @shoyer above. My comment seems still valid to me.

Why focus on 'if x is not supplied'? Why no equivalent 'if y is not supplied'? I feel like I'm missing something.

See the following example:

import xarray as xr import numpy as np lon, lat = np.meshgrid(np.linspace(-20, 20, 5), np.linspace(0, 30, 4)) da = xr.DataArray(np.arange(20).reshape(4, 5), dims=['y', 'x'], coords = {'lat': (('y', 'x'), lat), 'lon': (('y', 'x'), lon)}) da.plot(x='lat')

Note that da.plot(x='lat', y='lon') is a valid call and that 'lat' in da.coords is True (but 'lat' in da.dims is False).

With your current PR the code snippet above would lead to the following error:

x must be a coordinate variable

Which is misleading, since 'lat' is a coordinate variable (it is not a dimension coordinate, however).

I just opened an issue to discuss these terminology problems: #1295

Should we check for x and y in darray.coords then, instead of darray.dims?

No, I don't think this would work.

Since we will probably land on the name "coordinate variable" for the "dimension" coords, I guess we can go back to @shoyer 's initial suggestion, or change the error message to:

y must be a dimension name if x is not supplied

which has the advantage to cover all cases (also when there are unindexed dimensions).

So in your code sample above, for instance, we don't want to guess that y should be "lon"? Why not?

Ok, I understood :-)

Because in 2D Plots we are guaranteed to have only 2 dims, but we can have an arbitrary number of auxiliary coordinates, in an arbitrary order. In my example above checking for coords would work, but this is not always the case. See:

import xarray as xr import numpy as np lon, lat = np.meshgrid(np.linspace(-20, 20, 5), np.linspace(0, 30, 4)) da = xr.DataArray(np.arange(20).reshape(4, 5), dims=['y', 'x'], coords = {'lat': (('y', 'x'), lat), 'lon': (('y', 'x'), lon), 'x': (('x', ), np.arange(5)), 'y': (('y', ), np.arange(4)), }) print(da.coords) Coordinates: * y (y) int64 0 1 2 3 lon (y, x) float64 -20.0 -10.0 0.0 10.0 20.0 -20.0 -10.0 0.0 10.0 ... * x (x) int64 0 1 2 3 4 lat (y, x) float64 0.0 0.0 0.0 0.0 0.0 10.0 10.0 10.0 10.0 10.0 ...

Got it, thanks :-)

shoyer · 2017-03-04T02:27:57Z

Tests should pass after a rebase or merge thanks to #1293.

fmaussion · 2017-03-05T15:12:45Z

doc/whats-new.rst

@@ -22,6 +22,10 @@ v0.9.2 (unreleased)
 Enhancements
 ~~~~~~~~~~~~

+- When `plot()` is called on a 2D DataArray and only one dimension is
+  specified with `x=` or `=y`, the other dimension is now guessed. By


typo: =y -> y=

…ll to pcolormesh. Fix related tests

vnoel · 2017-03-06T15:46:23Z

I hope everything's ok now. Thanks for the hand-holding

shoyer

other than style, looks good to me

shoyer · 2017-03-06T17:23:30Z

xarray/tests/test_plot.py

            self.plotmethod('not_a_real_dim', 'y')
+        with self.assertRaisesRegexp(ValueError, 'x must be a dimension name if y is not supplied'):


I don't have a character count handy, but is this at 80 or more characters? If so, please split onto another line. I think flake8 or pep8 should catch this.

79 characters. flake8 reports lots of errors but not in this code.

Can you double check please? git diff upstream/master | flake8 --diff checks only your addition.

git had switched branch under me, sorry

fmaussion · 2017-03-07T14:54:14Z

Good, thanks!

Guess the complementary dimension when only one is passed to pcolormesh

4ec44bf

Add tests, verify the requested dimension is in the array

fb980a0

fix typo

1de63e3

shoyer reviewed Mar 3, 2017

View reviewed changes

vnoel added 2 commits March 4, 2017 10:14

updated whatsnew

a3e0b89

Forgot to add my name

ccb6b9c

fmaussion mentioned this pull request Mar 4, 2017

Terminology for the various coordinates #1295

Closed

fmaussion reviewed Mar 5, 2017

View reviewed changes

vnoel added 2 commits March 6, 2017 16:31

more accurate error message when specifying only one axis during a ca…

43e81fe

…ll to pcolormesh. Fix related tests

fix typo

1fd7319

fmaussion approved these changes Mar 6, 2017

View reviewed changes

shoyer approved these changes Mar 6, 2017

View reviewed changes

make style flake8-compliant

5911be9

fmaussion merged commit afbdbc7 into pydata:master Mar 7, 2017

vnoel deleted the pcolormesh-guess-dim branch March 7, 2017 15:46

stanwest mentioned this pull request Dec 16, 2020

Plotting in 2D with one non-dimension coordinate given behaves contrary to documentation and can cause cryptic ValueError #4702

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Guess the complementary dimension when only one is passed to pcolormesh #1291

Guess the complementary dimension when only one is passed to pcolormesh #1291

vnoel commented Mar 2, 2017 •

edited

Loading

fmaussion commented Mar 2, 2017

fmaussion commented Mar 2, 2017

vnoel commented Mar 2, 2017

fmaussion commented Mar 2, 2017

vnoel commented Mar 2, 2017 •

edited

Loading

vnoel commented Mar 3, 2017

shoyer commented Mar 3, 2017 •

edited

Loading

shoyer left a comment

shoyer Mar 3, 2017

vnoel Mar 4, 2017

fmaussion Mar 4, 2017

vnoel Mar 4, 2017

fmaussion Mar 4, 2017

fmaussion Mar 5, 2017

vnoel Mar 5, 2017

vnoel Mar 5, 2017

fmaussion Mar 5, 2017

vnoel Mar 5, 2017

shoyer commented Mar 4, 2017

fmaussion Mar 5, 2017

vnoel commented Mar 6, 2017

shoyer left a comment

shoyer Mar 6, 2017

vnoel Mar 7, 2017

fmaussion Mar 7, 2017 •

edited

Loading

vnoel Mar 7, 2017

fmaussion commented Mar 7, 2017

		self.plotmethod('not_a_real_dim', 'y')
		with self.assertRaisesRegexp(ValueError, 'x must be a dimension name if y is not supplied'):

Guess the complementary dimension when only one is passed to pcolormesh #1291

Guess the complementary dimension when only one is passed to pcolormesh #1291

Conversation

vnoel commented Mar 2, 2017 • edited Loading

fmaussion commented Mar 2, 2017

fmaussion commented Mar 2, 2017

vnoel commented Mar 2, 2017

fmaussion commented Mar 2, 2017

vnoel commented Mar 2, 2017 • edited Loading

vnoel commented Mar 3, 2017

shoyer commented Mar 3, 2017 • edited Loading

shoyer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shoyer commented Mar 4, 2017

Choose a reason for hiding this comment

vnoel commented Mar 6, 2017

shoyer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fmaussion Mar 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fmaussion commented Mar 7, 2017

vnoel commented Mar 2, 2017 •

edited

Loading

vnoel commented Mar 2, 2017 •

edited

Loading

shoyer commented Mar 3, 2017 •

edited

Loading

fmaussion Mar 7, 2017 •

edited

Loading