Reorganization of declarative syntax

[kgoebber]

Description Of Changes

Okay, so try number 2 to get the reorganization, started by @jthielen to allow for a more flexible syntax to add other types of plots (e.g., SkewT, Hovmoller, Line Plots, etc.). This is largely based on the reorganization that @jthielen produced and supplied in the discussion about changes to the declarative syntax.

In future PRs we'll work to add these new plotting abilities, which may then require further, albeit hopefully smaller changes, to the structure to allow for maximum functionality.

[dopplershift]

Doesn't this take us back to an era where we depended up on the order of the dimensions? Couldn't we pick one or the other of x and longitude?

[jthielen]

Good catch, I missed that in jthielen@cfef550. Though I do wonder based on not seeing any transpose/shape handling if xy-ordered data worked before this change? (Or I could just be totally missing something...I haven't worked with xy-ordered data in quite some time.)

However, we definitely can't just pick the physical x dimension coordinate, since this is an x axis only in the plotting sense, data-wise it could be any of MetPy's axis types (or even an arbitary dimension like the index of a cross section). In the case where the two dimensions are MetPy axis types, given that there are six possible dimension pairs (x-y, x-vertical, x-time, y-vertical, y-time, and vertical-time), should we force/assume which of the pair goes on which axis or find a way to leave it up to the user? Or find some compromise in between? And how should we handle arbitrary dimensions?

[kgoebber]

So in the end, we will always have two dimensional data, and we now have better checks for that with a warning. We could default to the ordering of the data based on our most common current use case (y-x) and build in some sensible choices based on the names of dimensions, but otherwise include an attribute like swap_axes that could be boolean (which defaults to False) to allow a change on the fly.

[kgoebber]

Okay, I have taken a swing at updating the reorganization to align the comments. It doesn't fully address @jthielen points around ordering of dimensions, but at least gets us set up to tackle that more robustly once we go to add our first non-xy (non-latlon) declarative plot. Will need to address the BarbPlot as well to be more robust with choice of dimension, currently set of a y-x plotting order.

[dopplershift]

Is there a reason not to do:

try:
    x = self.griddata.metpy.x
    y = self.griddata.metpy.y
except AttributeError:
    x = self.griddata[self.griddata.dims[1]]
    y = self.griddata[self.griddata.dims[0]]

return x, y, self.griddata

This would seem less likely to break anything (i.e. things that have an x/y coordinates without calling them lat/lon or x/y).

[kgoebber]

The code you suggested does work, but again, I think we are attempting to future proof here to make it easier to add different types of plots (and those some elif statements to handle other cases. @jthielen again might offer his own insight on this one too.

[jthielen]

@dopplershift We unfortunately can't do that since x and/or y from the accessor may still be present even when we are not in yx (or xy) space (likewise to the previous comment). But, yes, the implementation here is way to likely to break something (I honestly don't know why I even did it this way in the first place).

Like the other case, we'll probably need to leverage a try/except with find_axis_number:

try:
    plot_x_dim = self.griddata.metpy.find_axis_number('x')
    plot_y_dim = self.griddata.metpy.find_axis_number('y')
except ValueError:
    plot_x_dim = 1
    plot_y_dim = 0

return (
    self.griddata[self.griddata.dims[plot_x_dim]],
    self.griddata[self.griddata.dims[plot_y_dim]],
    self.griddata
)

[dopplershift]

I'm much happier with the implementation suggested by @jthielen here and below.

[dopplershift]

This doesn't seem right. x_like/y_like is relying on the order, while we then look at finding them by name, but then assume those are the same.

[kgoebber]

I think this is currently a future proofing set up to be able to plot wind barbs not just on XY planes, but time-height cross sections or regular cross sections. We don't currently have that ability, so that is why the if statement checking for matching of x and y coordinates doesn't have an elif statement at this time. @jthielen might be able to provide more clarity here.

[jthielen]

Yes, this was an attempt at future-proofing for non-map 2D plots. As described in the comment below, this is a conditional check to only carry out the earth/grid relative adjustments if we are in the plane of the plot (i.e., a map rather than a general 2D plot).

However, the confusion is definitely warranted, since this doesn't match the intent...or at the very least, won't work for the desired generalized case. Since find_axis_name only finds dimension coordinate names, as-is this will error out when not in an yx case, since then y and/or x are not there.

The goal is to check that we are in yx space (or technically also xy, but my initial attempt neglected the non-standard dimension order), which gets a bit tricky since we have to use find_axis_name or find_axis_number which enforce dimension coordinates rather than MetPy's coordinate helpers for y and x (which just enforce 0D or 1D coords). So, this may need to change to something like the following

check_earth_relative = False
try:
    plot_x_dim = self.griddata[0].metpy.find_axis_number('x')
    plot_y_dim = self.griddata[0].metpy.find_axis_number('y')
    check_earth_relative = True
except ValueError:
   plot_x_dim = 1
   plot_y_dim = 0

x_like = self.griddata[0][self.griddata[0].dims[plot_x_dim]]
y_like = self.griddata[0][self.griddata[0].dims[plot_y_dim]]

if check_earth_relative:
   ...

[kgoebber]

Okay, I have implemented both changes. Some of this may naturally evolve once we implement the first non-xy, non-map plotting through declarative, but that should all be on the backend and not expose too much tax on ourselves moving forward.

[dcamron]

Apologies for the silent lurking up to this point, but I appreciate this most recent discussion and these last few changes; I think it's ready to get in!