Skip to content
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

Convert obs units #1329

Merged
merged 3 commits into from Aug 8, 2020
Merged

Convert obs units #1329

merged 3 commits into from Aug 8, 2020

Conversation

kgoebber
Copy link
Collaborator

@kgoebber kgoebber commented Feb 28, 2020

Description Of Changes

This is an attempt to add an attribute to the declarative syntax to convert units if they are available in data object. So this will work if using the METAR parser or getting upper air observations from the Iowa State archive through Siphon. You can make it work through reading in another manner and adding in the appropriate units, but that takes some real effort with using the Pandas read_csv file.

Thoughts and feedback on the implementation are very welcome. The two current commits cover 1) adding a unit conversion to the StationPlot class and 2) adding the plot_units (for scalar quantities) and vector_plot_units (for vector quantities) as attributes to PlotObs.

Note: Missed some documentation in this initial PR for PlotObs().

Also did a slight refactor of PlotObs() to make better use of kwargs - don't know if that helps the code readability, but seemed like a good idea.

Checklist

  • Tests added
  • Fully documented

Copy link
Member

@dcamron dcamron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a helpful change and I think (almost) everything looks clean!

src/metpy/plots/declarative.py Outdated Show resolved Hide resolved
dcamron
dcamron previously approved these changes May 2, 2020
@dopplershift dopplershift added this to the 1.0 milestone Aug 6, 2020
Copy link
Member

@dopplershift dopplershift left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in really good shape. Just a minor change I noted.

raise ValueError('To convert to plotting units, units must be attached to '
'scalar value being converted.')

# Strip units, CartoPy transform doesn't like
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stripping units shouldn't be necessary, CartoPy only hates them for barbs because it does the extra work of re-projecting the vectors to the transformed coordinate system.

@dopplershift dopplershift added Area: Plots Pertains to producing plots Type: Enhancement Enhancement to existing functionality labels Aug 8, 2020
@dopplershift dopplershift merged commit 345ddd0 into Unidata:master Aug 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Plots Pertains to producing plots Type: Enhancement Enhancement to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants