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

precipitable water import #298

Merged
merged 9 commits into from
Jul 12, 2020
Merged

precipitable water import #298

merged 9 commits into from
Jul 12, 2020

Conversation

PharaohCola13
Copy link
Contributor

Description Of Changes

Updates to wyoming.py to allow Precipitable Water measurements to be pulled.

Checklist

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.

Thanks for the contribution! I can definitely see the use in having the precipitable water values that Wyoming provides available (I think we have ignored them in the past because we use MetPy to calculate). I just have a few questions/comments to address and we can get this in!

siphon/http_util.py Outdated Show resolved Hide resolved
siphon/simplewebservice/wyoming.py Outdated Show resolved Hide resolved
siphon/simplewebservice/wyoming.py Outdated Show resolved Hide resolved
@PharaohCola13
Copy link
Contributor Author

The suggestions that were laid were some cheap workarounds for local testing/running. Those changes have been adjusted such that the only real alteration is the inclusion of precipitable water in the data pull, as that was the goal.

Thank you

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.

Just one minor bit of lint to satisfy the bots, then this is ready to go.

I'm not sure what the deal with coverage is, but your changes look just fine.

siphon/http_util.py Outdated Show resolved Hide resolved
@dopplershift
Copy link
Member

If you want bonus points, you could test that the value of precipitable water is properly decoded here:

assert_almost_equal(df['pressure'][5], 867.9, 2)
assert_almost_equal(df['height'][5], 1219., 2)
assert_almost_equal(df['temperature'][5], 17.4, 2)
assert_almost_equal(df['dewpoint'][5], 14.3, 2)
assert_almost_equal(df['u_wind'][5], 6.60, 2)
assert_almost_equal(df['v_wind'][5], 37.42, 2)
assert_almost_equal(df['speed'][5], 38.0, 1)
assert_almost_equal(df['direction'][5], 190.0, 1)

@PharaohCola13
Copy link
Contributor Author

I can take a look and integrate precipitable water into the tests.

@CLAassistant
Copy link

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor Author

@PharaohCola13 PharaohCola13 left a comment

Choose a reason for hiding this comment

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

Precipitable water can be directly imported from the changes made

Copy link
Contributor Author

@PharaohCola13 PharaohCola13 left a comment

Choose a reason for hiding this comment

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

Resolved

Copy link
Contributor Author

@PharaohCola13 PharaohCola13 left a comment

Choose a reason for hiding this comment

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

Resolved

Fixed a commit conflict previously missed
Copy link
Contributor Author

@PharaohCola13 PharaohCola13 left a comment

Choose a reason for hiding this comment

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

Resolved a units issue

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.

Looks good! Thanks for the contribution!

@dopplershift dopplershift merged commit bd972b4 into Unidata:master Jul 12, 2020
@dopplershift dopplershift added this to the 0.9 milestone Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants