Move upper air data capability into Siphon #147

jrleeman · 2017-08-23T15:24:35Z

Addresses #130 and moves upper air data support into Siphon. Still a sketch given that we have some questions on where exactly this should live. I've used the in memory capability of netCDF4-python here, but without wrapping the Variable like we do in MetPy, units are a no go. Then again, we'd be depending on MetPy's unit registry at that point, so maybe we return a plain netCDF Dataset? I'm thinking a tool in MetPy that goes through and deals with appending units from metadata might be useful anyway.

lesserwhirls · 2017-08-23T15:57:51Z

siphon/upperair.py

+    -------
+        :class:`metpy.io.cdm.Dataset` containing the data
+
+    .. deprecated:: 0.6.0


Do we need this warning mirrored in siphon?

Nope, we can get rid of it.

lesserwhirls · 2017-08-23T16:02:24Z

Do we have tests in MetPy that we can move over to Siphon?

jrleeman · 2017-08-23T16:08:56Z

There are some tests that will need to be re-mocked. Especially my test on server return codes.

jrleeman

A few places I need to update or would like input on @dopplershift

jrleeman · 2017-08-24T15:24:21Z

siphon/simplewebservice/wyoming.py

+from io import BytesIO
+import numpy as np
+import pandas as pd
+from metpy.calc import get_wind_components


Planning on porting get_wind_components from metpy into an _tools.

jrleeman · 2017-08-24T15:25:34Z

siphon/simplewebservice/wyoming.py

+        :class:`pandas.DataFrame` containing the data
+
+    """
+    endpoint = WyomingUpperAir('http://weather.uwyo.edu/')


I don't like the URL here, I'm feeling like the object setup isn't ideal here.

Put it in the constructor for the class...see below.

jrleeman · 2017-08-24T15:26:11Z

siphon/simplewebservice/wyoming.py

+                      ('td', 'dewpoint')]:
+        key_data, key_units = info[key]
+
+        insert_with_units(data, name, key_data, key_units)


Need to add u, v, speed, direction still.

dopplershift

Here are my thoughts. You might also look at ncss.py to see how I did things there.

dopplershift · 2017-08-24T16:31:38Z

siphon/simplewebservice/wyoming.py

+
+class WyomingUpperAir(HTTPEndPoint):
+    """Download and parse data from the University of Wyoming's upper air archive."""
+


I think the way to go is to add a __init__ that takes no arguments and passes the url to:

super(WyomingUpperAir, self).__init__('myurlhere')

Should also include 'cgi-bin/sounding' in that url.

dopplershift · 2017-08-24T16:34:48Z

siphon/simplewebservice/wyoming.py

+        a file-like object from which to read the data
+
+        """
+        path = ('cgi-bin/sounding?region={region}&TYPE=TEXT%3ALIST'


Let's encompass this in a Query object for Wyoming. Might need to refactor DataQuery to allow a query object for this to inherit useful behavior.

dopplershift · 2017-08-24T16:34:58Z

siphon/upperair.py

@@ -0,0 +1,279 @@
+# Copyright (c) 2013-2015 University Corporation for Atmospheric Research/Unidata.


I assume this file is going away.

dopplershift · 2017-08-24T16:36:08Z

siphon/simplewebservice/wyoming.py

+                '&YEAR={time:%Y}&MONTH={time:%m}&FROM={time:%d%H}&TO={time:%d%H}'
+                '&STNM={stid}').format(region=region, time=time, stid=site_id)
+
+        resp = self.get_path(path)


With everything above, this becomes self.get_query(query).

dopplershift · 2017-08-24T16:38:31Z

siphon/simplewebservice/wyoming.py

+
+        # Grab the stuff *between* the <PRE> tags -- 6 below is len('<PRE>\n')
+        buf = resp.text[data_start + 6:data_end]
+        return BytesIO(buf.encode(encoding='UTF-8'))


Can probably just move the contents of parse here since we're not trying to have the same common structure as we did in MetPy. Might be able to leverage pandas' own parsing code to do this work too.

dopplershift · 2017-08-24T16:40:18Z

siphon/simplewebservice/wyoming.py

+        :class:`pandas.DataFrame` containing the data
+
+    """
+    endpoint = WyomingUpperAir('http://weather.uwyo.edu/')


Put it in the constructor for the class...see below.

dopplershift · 2017-08-24T16:40:48Z

siphon/simplewebservice/wyoming.py

+    fobj = endpoint.get_data(time, site_id)
+    info = endpoint.parse(fobj)
+
+    direction, spd, spd_units = info['wind']


I think all of the pandas manipulation should be in get_data in the WyomingUpperAir class.

dopplershift · 2017-08-24T16:41:40Z

siphon/simplewebservice/wyoming.py

+from metpy.calc import get_wind_components
+
+
+def get_upper_air_data(time, site_id):


I think:

This function should be a staticmethod on WyomingUpperAir

It should just wrap the process of building a query and calling get_data.

dopplershift · 2017-08-24T16:43:12Z

Happy to discuss more...probably easier to get into details now that you have a concrete picture.

jrleeman · 2017-08-24T22:08:21Z

Will need to have a discussion about using query tomorrow, but I have reworked the parsing completely and it's now pretty robust as far as I can tell.

CLAassistant · 2017-09-12T13:25:06Z

All committers have signed the CLA.

jrleeman · 2017-09-12T14:21:37Z

Ready for re-review @dopplershift

jrleeman · 2017-09-12T15:16:07Z

Also notice that some of the indexes in the test changed from MetPy - we're returning the full data frame, even if the first rows (lowest altitudes) are all NaN's. I think that's fine, but am up for disagreement.

dopplershift · 2017-09-12T16:40:56Z

Is there an easy way with pandas to remove empty rows?

jrleeman · 2017-09-12T18:41:57Z

Technically not an empty row as the populate height still.... I'll see if there is an easy way to cut if all other fields are NaN

jrleeman · 2017-09-12T19:54:21Z

See what you think about this. If we like it, I'll fixup. Not the nicest solution, but the end result is nice. You must drop the row only when all the columns specified are NaN, then reindex so the index column starts a zero and is continuous.

dopplershift · 2017-09-12T20:12:09Z

I'm curious why you don't think it's a nice solution? It looks like a perfectly straightforward one-liner to me.

jrleeman · 2017-09-12T20:19:53Z

Seems verbose for a simple task. i.e. manually specifying the index rest and needed to give a kwarg to prevent maintaining the old index as a column. It is a one-liner, just seemed overly long for a simple task.

dopplershift · 2017-09-12T20:22:48Z

Me thinks someone's standards are a tad high. 😈 Lest I remind you this is how we did this before:

            if any(np.invert(np.isnan(values[1:]))):
                arr_data.append((level,) + values)

Not exactly the bastion of simple code either.

jrleeman · 2017-09-12T20:24:05Z

Don't burst my idealistic bubble 😄 - just fixed up, pending CI, we're good to go.

dopplershift · 2017-09-12T20:25:16Z

I would have figured that bubble had been burst many times over the last 6 months. 😁

jrleeman requested review from dopplershift and lesserwhirls as code owners August 23, 2017 15:24

lesserwhirls approved these changes Aug 23, 2017

View reviewed changes

jrleeman force-pushed the Upperair_Data branch from 06e80fb to 75e186e Compare August 23, 2017 16:16

jrleeman commented Aug 24, 2017

View reviewed changes

dopplershift requested changes Aug 24, 2017

View reviewed changes

jrleeman force-pushed the Upperair_Data branch from 00309ac to 7996409 Compare August 24, 2017 22:06

dopplershift modified the milestone: 0.6 Sep 6, 2017

jrleeman force-pushed the Upperair_Data branch from 7996409 to a2ccda3 Compare September 12, 2017 13:24

jrleeman force-pushed the Upperair_Data branch 2 times, most recently from 81f0815 to 058b6ac Compare September 12, 2017 13:46

jrleeman force-pushed the Upperair_Data branch from a897445 to 713f769 Compare September 12, 2017 19:57

jrleeman added 4 commits September 12, 2017 14:22

Add Wyoming upper air capability.

ba1ad91

Move get_wind_components to _tools

d76f520

Add tests for Wyoming upper air.

4d41875

Add beautifulsoup4 as a dependency

7c8e887

jrleeman force-pushed the Upperair_Data branch from 713f769 to 7c8e887 Compare September 12, 2017 20:23

dopplershift approved these changes Sep 12, 2017

View reviewed changes

dopplershift merged commit 259ee92 into Unidata:master Sep 12, 2017

dopplershift mentioned this pull request Sep 12, 2017

Port upper air support from MetPy #130

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move upper air data capability into Siphon #147

Move upper air data capability into Siphon #147

jrleeman commented Aug 23, 2017

lesserwhirls Aug 23, 2017

jrleeman Aug 23, 2017

lesserwhirls commented Aug 23, 2017

jrleeman commented Aug 23, 2017

jrleeman left a comment

jrleeman Aug 24, 2017

jrleeman Aug 24, 2017

dopplershift Aug 24, 2017

jrleeman Aug 24, 2017

dopplershift left a comment

dopplershift Aug 24, 2017

dopplershift Aug 24, 2017

dopplershift Aug 24, 2017

dopplershift Aug 24, 2017

dopplershift Aug 24, 2017

dopplershift Aug 24, 2017

dopplershift Aug 24, 2017

dopplershift Aug 24, 2017

dopplershift commented Aug 24, 2017

jrleeman commented Aug 24, 2017

CLAassistant commented Sep 12, 2017 •

edited

Loading

jrleeman commented Sep 12, 2017

jrleeman commented Sep 12, 2017

dopplershift commented Sep 12, 2017

jrleeman commented Sep 12, 2017

jrleeman commented Sep 12, 2017

dopplershift commented Sep 12, 2017

jrleeman commented Sep 12, 2017

dopplershift commented Sep 12, 2017

jrleeman commented Sep 12, 2017

dopplershift commented Sep 12, 2017


		class WyomingUpperAir(HTTPEndPoint):
		"""Download and parse data from the University of Wyoming's upper air archive."""

		@@ -0,0 +1,279 @@
		# Copyright (c) 2013-2015 University Corporation for Atmospheric Research/Unidata.

		from metpy.calc import get_wind_components


		def get_upper_air_data(time, site_id):

Move upper air data capability into Siphon #147

Move upper air data capability into Siphon #147

Conversation

jrleeman commented Aug 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lesserwhirls commented Aug 23, 2017

jrleeman commented Aug 23, 2017

jrleeman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dopplershift left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dopplershift commented Aug 24, 2017

jrleeman commented Aug 24, 2017

CLAassistant commented Sep 12, 2017 • edited Loading

jrleeman commented Sep 12, 2017

jrleeman commented Sep 12, 2017

dopplershift commented Sep 12, 2017

jrleeman commented Sep 12, 2017

jrleeman commented Sep 12, 2017

dopplershift commented Sep 12, 2017

jrleeman commented Sep 12, 2017

dopplershift commented Sep 12, 2017

jrleeman commented Sep 12, 2017

dopplershift commented Sep 12, 2017

CLAassistant commented Sep 12, 2017 •

edited

Loading