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

Update to Treat RH as a Unitless Ratio in Existing Functions #677

Merged
merged 2 commits into from Dec 13, 2017
Merged

Update to Treat RH as a Unitless Ratio in Existing Functions #677

merged 2 commits into from Dec 13, 2017

Conversation

jthielen
Copy link
Collaborator

@jthielen jthielen commented Dec 9, 2017

As discussed in PR #669, for consistency with the rest of the library, relative humidity should be treated as a unitless ratio in [0, 1]. It was noticed that some previous functions in thermo.py treated it as a percentage. This aims to correct that.

As discussed in PR #669, for consistency with the rest of the library, relative humidity should be treated as a unitless ratio in [0, 1]. It was noticed that some previous functions in thermo.py treated it as a percentage. This aims to correct that.
@dopplershift
Copy link
Member

Looks like this needs some updates to some existing tests in order to pass. Are you interested in fixing those?

@dopplershift dopplershift added Area: Calc Pertains to calculations Type: API Change Changes to how existing functionality works Type: Enhancement Enhancement to existing functionality labels Dec 11, 2017
@dopplershift dopplershift added this to the 0.7 milestone Dec 11, 2017
@jthielen
Copy link
Collaborator Author

Yes, I would be interested, but I am uncertain as to what needs to be done. Would you be able to let me know what tests are failing where? (All tests with py.test are passing for me locally on Python 3.6.3)

@dopplershift
Copy link
Member

I retriggered AppVeyor and Travis to see if it was just a transient error. If not, we'll dig in and point you in the right direction.

Thanks for letting us know that the tests pass locally for you.

@dopplershift
Copy link
Member

So as you would <sarcasm> clearly expect </sarcasm> from this error message:

Traceback (most recent call last):
  File "isentropic_example.py", line 156, in <module>
    cmap=plt.cm.gist_earth_r)
  File "/Users/rmay/miniconda3/envs/py36/lib/python3.6/site-packages/cartopy/mpl/geoaxes.py", line 1338, in contourf
    for col in result.collections
  File "/Users/rmay/miniconda3/envs/py36/lib/python3.6/site-packages/matplotlib/transforms.py", line 724, in union
    raise ValueError("'bboxes' cannot be empty")
ValueError: 'bboxes' cannot be empty

The problem is in this line:

#################################
# **Converting to Relative Humidity**
#
# The NARR only gives specific humidity on isobaric vertical levels, so relative humidity will
# have to be calculated after the interpolation to isentropic space.
isentrh = mcalc.relative_humidity_from_specific_humidity(isentspech, isenttmp, isentprs)
#######################################

The contourf calls later have intervals that expect percent values, and they care not at all about units. If you could change this line to multiply by 100, that will fix the problem.

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 need to fix the example.

@jthielen
Copy link
Collaborator Author

@dopplershift For future reference, would this sort of issue be something I could catch locally if I tried building the docs, and not just doing flake8 and py.test?

@dopplershift
Copy link
Member

Yes, that would do it. If you have all of the dependencies (like cartopy), it should be as straightforward as doing:

cd docs
make html

That should work even on windows. But there's nothing wrong with not doing that and letting our CI (continuous integration) setup catch those kind of things--it's just slower than running locally.

@dopplershift dopplershift merged commit f40cea0 into Unidata:master Dec 13, 2017
@dopplershift
Copy link
Member

Awesome! Thanks for more good work.

@jthielen jthielen deleted the rh_unitless_conversion branch December 14, 2017 04:14
@jthielen jthielen restored the rh_unitless_conversion branch January 17, 2018 02:20
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: Enhancement Enhancement to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants