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

Raise exception if LCL does not converge #328

Merged
merged 2 commits into from Feb 28, 2017

Conversation

jrleeman
Copy link
Contributor

If we don't converge in max_iter we should fail loudly. Addresses #317

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.

Could you add a test by setting max_iters=1? It'd be nice to have that for many reasons.

@dopplershift
Copy link
Member

You'll need:

with pytest.raises(RuntimeError):
   lcl(...)

@@ -159,6 +160,11 @@ def test_lcl():
assert_almost_equal(lcl_pressure, 864.761 * units.mbar, 2)
assert_almost_equal(lcl_temperature, 17.676 * units.degC, 2)

Copy link
Member

Choose a reason for hiding this comment

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

pep8 == two blank lines

def test_lcl_convergence():
"""Test LCL calculation convergence failure."""
with pytest.raises(RuntimeError):
lcl_pressure, lcl_temperature = lcl(1000. * units.mbar, 30. * units.degC,
Copy link
Member

Choose a reason for hiding this comment

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

Don't even bother capturing the return values.

"""Test LCL calculation convergence failure."""
with pytest.raises(RuntimeError):
lcl_pressure, lcl_temperature = lcl(1000. * units.mbar, 30. * units.degC,
20. * units.degC, max_iters=2)

Copy link
Member

Choose a reason for hiding this comment

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

Another blank here for pep8.

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.

Pending a clean CI run.

@dopplershift
Copy link
Member

I will merge despite being "out of date" since the last PR to go in was just docs--so long as this passes cleanly.

@dopplershift
Copy link
Member

Appveyor failure was spurious...restarted.

@dopplershift dopplershift merged commit 0c8cf26 into Unidata:master Feb 28, 2017
@jrleeman jrleeman deleted the LCL_Convergence branch February 28, 2017 16:20
@dopplershift dopplershift modified the milestone: Spring 2017 Mar 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants