-
Notifications
You must be signed in to change notification settings - Fork 441
Height Scale for SkewT #3853
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
base: main
Are you sure you want to change the base?
Height Scale for SkewT #3853
Conversation
|
Jaye Norman seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
src/metpy/plots/skewt.py
Outdated
| def __init__(self, fig=None, rotation=30, subplot=None, rect=None, | ||
| aspect=80.5, show_heights=False): |
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 would you feel about this interface,
skew = metpy.calc.SkewT(show_heights=True)vs
skew = metpy.calc.SkewT()
skew.heightax()
# or
skew.add_heightax()
# or
skew.add_height_scale()etc etc etc., or something like that? To me, a height axis almost exactly straddles the line of "plot config" vs "physical parameter", so I'm torn, personally.
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.
@dopplershift Mentioned on the original issue:
I'd really like to have it separated from the plotting call
In an ideal world, it'd be dynamically updated from the y-axis in pressure coords. I'm not sure if a
matplotlib spine is enough, or if maybe we do need the separate Axes, but we use matplotlib's event
handling to update the range on the height scale
Which to me points to a separate method like add_heightax(). I originally interpreted the issue this way because a height axis feels a bit more like a "plot config" but in theory it shouldn't be hard to switch it around; the issue will be making sure it can dynamically update. I'll bake this up (maybe in a separate branch for now) and see what it entails
src/metpy/plots/skewt.py
Outdated
| Aspect ratio (i.e. ratio of y-scale to x-scale) to maintain in the plot. | ||
| Defaults to 80.5. Passing the string ``'auto'`` tells matplotlib to handle | ||
| the aspect ratio automatically (this is not recommended for SkewT). | ||
| show_heights : boolean, optional |
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.
| show_heights : boolean, optional | |
| show_heights : bool, optional |
if we keep this interface, this should fix the doc builds yelling at you!



Description Of Changes
Added new argument to metpy.plot.SkewT function for an optional secondary y-axis of height calculated from metpy.calc.pressure_to_height_std(). Defaults to 'False'; axis can be accessed as a matplotlib axis using 'heightax'.
Checklist