-
Notifications
You must be signed in to change notification settings - Fork 413
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
Hodograph Layer Coloring #482
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious how the UI of this compared to a segmented color map looks. Maybe this should be here, but us a segmented color map under the hood? @dopplershift likely has an opinion on this.
metpy/plots/tests/test_skewt.py
Outdated
@pytest.mark.mpl_image_compare(tolerance=0, remove_text=True) | ||
def test_hodograph_plot_layer(): | ||
"""Test hodograph colored layers.""" | ||
u = np.arange(5., 65., 5) * units('knot') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decimals after values here and in the test data shouldn't be necessary as we don't need to force them to be floats.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed this.
@@ -175,3 +175,23 @@ def test_skewt_barb_color(): | |||
skew.plot_barbs(p, u, u, c=u) | |||
|
|||
return fig | |||
|
|||
|
|||
@pytest.mark.mpl_image_compare(tolerance=0, remove_text=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will likely have to bump depending on Appveyor/Travis.
metpy/plots/skewt.py
Outdated
""" | ||
line_args = self._form_line_args(kwargs) | ||
u, v = delete_masked_points(u, v) | ||
for i in range(bounds.shape[0] - 1): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in range(len(bounds) -1)
should work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also fixed this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can simplify and make plot_colormapped
polymorphic here. Even though the snippet is only a couple of lines, this is a common enough use-case that it's boiler plate we don't want the user to need to write.
metpy/plots/skewt.py
Outdated
|
||
""" | ||
line_args = self._form_line_args(kwargs) | ||
u, v = delete_masked_points(u, v) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can condense this and ditch the loop :) I think we should make plot_colormapped
polymorphic. You can accept either a color map for continuous coloring or accept a bounds
and colors
kwarg to do a segmented coloring. Here's the snippet:
import matplotlib as mpl
# Get data, etc.
cmap = mpl.colors.ListedColormap(['r', 'g', 'b'])
norm = mpl.colors.BoundaryNorm([0, 1, 3, 5], cmap.N)
h = Hodograph(component_range=80.)
h.plot_colormapped(u, v, hgt, cmap=cmap)
h.add_grid(increment=20
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this actually using height values for the bounds here? I just tried this and it plots the same segments no matter what I set as the norm (or even if I comment out the norm). It appears to just be breaking the hodograph into three arbitrary segments without regard to the heights I give it, but I'm guessing I might just be missing something in my code.
fc39f71
to
bcdbed2
Compare
bcdbed2
to
9a0887c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we keeping both functions? Not sure why we need plot_layers
if plot_colormapped
has the same functionality.
metpy/plots/skewt.py
Outdated
@@ -770,10 +771,70 @@ def plot(self, u, v, **kwargs): | |||
u, v = delete_masked_points(u, v) | |||
return self.ax.plot(u, v, **line_args) | |||
|
|||
def plot_colormapped(self, u, v, c, **kwargs): | |||
def plot_layers(self, u, v, hgt, bounds, colors, **kwargs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should make naming consistent with get_layer
(i.e. heights)
metpy/plots/skewt.py
Outdated
|
||
Parameters | ||
---------- | ||
u : array_like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of these are pint.Quantity
metpy/plots/skewt.py
Outdated
bounds = np.asarray(bounds + hgt[0]) * bounds.units | ||
interp_vert = interp(bounds, hgt, hgt, u, v) | ||
inds = np.searchsorted(hgt.magnitude, bounds.magnitude) | ||
u = np.insert(u.magnitude, inds, interp_vert[1].magnitude) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be more efficient to insert the bounds into heights and then interp? That way you'd get all the points plus the new ones for u and v from interp
.
metpy/plots/skewt.py
Outdated
@@ -770,10 +771,70 @@ def plot(self, u, v, **kwargs): | |||
u, v = delete_masked_points(u, v) | |||
return self.ax.plot(u, v, **line_args) | |||
|
|||
def plot_colormapped(self, u, v, c, **kwargs): | |||
def plot_layers(self, u, v, hgt, bounds, colors, **kwargs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this function sticking around after the functionality was implanted in plot_colormapped
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They've got slightly different functionalities. Plot_layers is designed specifically for layers in height coordinates and will interpolate to find the exact bounds, while the addition to plot_colormapped will color the hodograph by any arbitrary third variable without interpolating to the bounds. This difference is necessary because searchsorted doesn't work well with data that aren't increasing or decreasing monotonically, so adding the interpolated points when coloring the hodograph by temperature or wind speed doesn't work like it does with height.
9a0887c
to
d3bf290
Compare
d3bf290
to
9f4452a
Compare
@dopplershift - can you redeliver to CLA Hub and LGTM? Looks like they got stuck. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the approach. Just some things to improve in the docs mainly.
metpy/plots/skewt.py
Outdated
Plots the wind data on the hodograph, but with a colormapped line. Takes a third | ||
variable besides the winds and either a colormap to color it with or a series of | ||
bounds and colors to create a custom colormap out of. The bounds must always be in | ||
increasing order or matplotlib is not happy. For using custom bounds with height data, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd remove "or matplotlib is not happy"
r"""Plot u, v data, with line colored based on a third set of data. | ||
|
||
Plots the wind data on the hodograph, but with a colormapped line. | ||
Plots the wind data on the hodograph, but with a colormapped line. Takes a third | ||
variable besides the winds and either a colormap to color it with or a series of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"...colormap and norm to control colormapping..."
metpy/plots/skewt.py
Outdated
@@ -787,6 +793,10 @@ def plot_colormapped(self, u, v, c, **kwargs): | |||
v-component of wind | |||
c : array_like | |||
data to use for colormapping | |||
bounds: array-like | |||
Array of bounds for c to use in coloring the hodograph. | |||
colors: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type of colors is "list". Should also note that it's optional.
metpy/plots/skewt.py
Outdated
@@ -787,6 +793,10 @@ def plot_colormapped(self, u, v, c, **kwargs): | |||
v-component of wind | |||
c : array_like | |||
data to use for colormapping | |||
bounds: array-like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bounds is optional.
metpy/plots/skewt.py
Outdated
@@ -7,6 +7,7 @@ | |||
`SkewT`, as well as a class for making a `Hodograph`. | |||
""" | |||
|
|||
import matplotlib as mpl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about import matplotlib.colors as mcolors
? That would be more standard with how I usually do it.
0470f75
to
da1808e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor doc style nits.
metpy/plots/skewt.py
Outdated
@@ -787,6 +794,10 @@ def plot_colormapped(self, u, v, c, **kwargs): | |||
v-component of wind | |||
c : array_like | |||
data to use for colormapping | |||
bounds: (optional) array-like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"array-like, optional"
metpy/plots/skewt.py
Outdated
@@ -787,6 +794,10 @@ def plot_colormapped(self, u, v, c, **kwargs): | |||
v-component of wind | |||
c : array_like | |||
data to use for colormapping | |||
bounds: (optional) array-like | |||
Array of bounds for c to use in coloring the hodograph. | |||
colors: (optional) list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same.
da1808e
to
8307dc4
Compare
@jrleeman good? (Travis failure is from egghead) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good now!
Uses the get_layer function to create a helper to plot different layers of a hodograph in different colors.