Improved formatting of notes section of METAR parser #1908
Conversation
There was a problem hiding this comment.
This looks really good (and a lot more readable)! This is an opportunity to fix up some inconsistent things in here too: skyc1, wind_spd actually come out in the dataframe as low_cloud_type, wind_speed in the returned columns of df (there are others too.) We also add a few new columns before returning, only one of which is (incorrectly) mentioned in the docstring. You could dig through those a bit and get the names -> columns -> docs all lined up. I'd also invite you to make the same formatting change for parse_metar_to_dataframe. Thanks for putting this together!
23ccozad
force-pushed
the
metar_parser_docs
branch
from
9066fde
to
d9c7ef2
Compare
|
Docs should now be all lined up for |
src/metpy/io/metar.py
Outdated
| * 'dew_point_temperature': Dew point, measured in degrees Celsius | ||
| * 'altimeter': Altimeter value, measured in inches of mercury, float | ||
| * 'present_weather': Current weather symbol, integer | ||
| * 'past_weather': Past weather (1 of 2), integer |
There was a problem hiding this comment.
This should be current_wx1_symbol - looks like I missed getting these changed in the PR that I did to modify the naming convention to be inline with what we said should be coming out of the METAR parser. So we'll want to change that here so that present_weather becomes current_wx1_symbol, past_weather becomes current_wx2_symbol, and past_weather2 becomes current_wx3_symbol.
src/metpy/io/metar.py
Outdated
| * 'cloud_coverage': Cloud cover measured in oktas, taken from maximum of sky cover values | ||
| * 'air_temperature': Temperature, measured in degrees Celsius | ||
| * 'dew_point_temperature': Dew point, measured in degrees Celsius | ||
| * 'altimeter': Altimeter value, measured in inches of mercury, float |
There was a problem hiding this comment.
Let's remove the float here and under air_pressure_at_sea_level since they're the only ones that list it. I doubt it's helping anyone.
There was a problem hiding this comment.
(and of course in the other places as well)
src/metpy/io/metar.py
Outdated
| * 'air_temperature': Temperature, measured in degrees Celsius | ||
| * 'dew_point_temperature': Dew point, measured in degrees Celsius | ||
| * 'altimeter': Altimeter value, measured in inches of mercury, float | ||
| * 'current_wx1_symbol': Current weather symbol (1 of 3), integer |
There was a problem hiding this comment.
How about instead of just integer, we go with WMO integer code and reference WMO 306 Vol 1 Part A (Attachment IV)? Would also be good to update the our existing reference since it looks like they moved it on us?
There was a problem hiding this comment.
(and of course in the other places as well)
23ccozad
force-pushed
the
metar_parser_docs
branch
from
84ec7b6
to
94067da
Compare
…nctions; added WMO No.306 reference
Updates parse_metar_to_dataframe() to return a dataframe with columns named current_wx1_symbol, current_wx2_symbol, and current_wx3_symbol, instead of current_weather, past_weather, and past_weather2
23ccozad
force-pushed
the
metar_parser_docs
branch
from
94067da
to
17e2857
Compare
|
Not quite sure why flake8 doesn't like my usage of |
|
If you look at |
Introduces bullet points in notes section of
parse_metar_file()docs, making it much easier to read.Old formatting of notes section for parse_metar_file():
Improved formatting with this PR:
Checklist