search

NCAR / wrf-python

A collection of diagnostic and interpolation routines for use with output from the Weather Research and Forecasting (WRF-ARW) Model.
https://wrf-python.readthedocs.io
Apache License 2.0
411 stars 155 forks source link

Correction of the 2nd example in 'interplevel' docstring #125

Closed pgamez closed 4 years ago

pgamez commented 4 years ago

Fixes issue #124

michaelavs commented 4 years ago

Before approving this PR for merge, would you mind running your code using height_agl instead of the z variable in your getvar() call? I checked the wrfout_d02_2010-06-13_21:00:00 file (used in the example) using both variables and found that the msl=False version matches with the height_agl version. If this is a consistent finding with your projection, I think it may be better to edit the example to use something like height = getvar(wrfin, "height_agl") instead of z = getvar(wrfin, "z", msl=False). Let me know your thoughts on this change.

pgamez commented 4 years ago

Hi Michaela.

I think you are right, height_agl would be the best option, definitively. It is more straightforward, more readable and would avoid any confusion. But that variable is not included in the current release (installed from pip). That's the reason why I dropped height_agl:

Traceback (most recent call last):
  File "./test.py", line 26, in <module>
    height = getvar(wrfin, "height_agl")
  File "/home/pgamez/.local/lib/python3.8/site-packages/wrf/routines.py", line 343, in getvar
    raise ValueError("'%s' is not a valid variable name" % (varname))
ValueError: 'height_agl' is not a valid variable name

However, I see that it works in the develop branch, so it is OK, and I am going to commit the change....

michaelavs commented 4 years ago

Thank you for making that change, I will approve this PR and merge it. On the subject of the version used not using height_agl, I will most likely add a note to the user on the webpage summarizing that there is another way to get the height variable which would be using z = getvar(wrfin, "z", msl=False) in case someone else has this issue in the future.

pgamez commented 4 years ago

Thank you very much Michaela! btw I have found another issue, so I'm going to open a new one... Greetings