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

Storm Relative Helicity Improvements #577

Merged
merged 7 commits into from Oct 19, 2017

Conversation

jrleeman
Copy link
Contributor

@jrleeman jrleeman commented Oct 3, 2017

Closes #576 - There was complication with get_layer and some non-ideal testing/methods. This PR cleans up the docs, adds the function get_layer_height for height only subsetting (get_layer requires and interpolates in pressure space only).

Testing now uses a triangular hodograph with a simple solution.

We may need to do some refactoring of get_layer, but that likely doesn't belong here. Either way I'm sure we can still do some API tuning.

@jrleeman jrleeman added this to the 0.6.1 milestone Oct 3, 2017
heights, data = get_layer_heights(heights, 500 * units.m, data, with_agl=True,
interpolation=False, bottom=200 * units.m)
heights_true = np.array([0.2, 0.3, 0.4, 0.5, 0.6, 0.7]) * units.km
data_true = np.array([50, 60 , 70, 80, 90, 100]) * units.degC

Choose a reason for hiding this comment

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

E203 whitespace before ','

heights_true = np.array([0.2, 0.3, 0.4, 0.5, 0.6, 0.7]) * units.km
data_true = np.array([50, 60 , 70, 80, 90, 100]) * units.degC
assert_array_almost_equal(heights_true, heights, 6)
assert_array_almost_equal(data_true, data, 6)

Choose a reason for hiding this comment

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

W292 no newline at end of file

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.

Minor doc cleanup. Also, why remove the original test? Are we still passing what it tested?

*args : array-like
Atmospheric variable(s) measured at the given pressures
bottom : `pint.Quantity`, optional
The bottom of the layer
Copy link
Member

Choose a reason for hiding this comment

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

Defaults to?

@jrleeman
Copy link
Contributor Author

jrleeman commented Oct 9, 2017

Safe to ignore code climate on this one I believe.

dopplershift
dopplershift previously approved these changes Oct 17, 2017
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.

Goodie, an API break in a bug fix release. Good to go I think once you get PEP8 clean.

@jrleeman
Copy link
Contributor Author

Multiprocessing issues within flake8?

@dopplershift
Copy link
Member

See #588. Once that goes green, you can rebase on it.

@jrleeman
Copy link
Contributor Author

Should be good to go pending LGTM

@dopplershift dopplershift merged commit 3232905 into Unidata:master Oct 19, 2017
@dopplershift dopplershift added Area: Calc Pertains to calculations Type: API Change Changes to how existing functionality works Type: Bug Something is not working like it should labels Oct 19, 2017
@jrleeman jrleeman deleted the SRH_Bounds branch October 19, 2017 17:02
@jrleeman
Copy link
Contributor Author

I know @mwilson14 was planning on a PR wrt this function, heads up that it's changed up some now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Calc Pertains to calculations Type: API Change Changes to how existing functionality works Type: Bug Something is not working like it should
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants