ioos / system-test

IOOS DMAC System Integration Test project
github.com/ioos/system-test/wiki
The Unlicense
7 stars 14 forks source link

coops2df function in utilities.py fails if water surface height above reference datum not in SOS observed properties #144

Open benjwadams opened 10 years ago

benjwadams commented 10 years ago

The function coops2df contained in a number of instances of utilities.py fails if given a collector with a station where 'water_surface_height_above_reference_datum (m)' is not an observedProperty. The failure occurs due to an uncaught KeyError exception which occurs when attempting to access the column of the Pandas dataframe generated from the CSV output of the SOS.

There is also a no-op if statement which will never be executed in the code -- if False:

Additionally, due to the try block in the code, if any OWSLib routines encounter an OWS exception while the collector.raw method is being called, a misleading error message could be returned indicating that there is no vertical datum being returned when in fact another OWS Exception occurred.

In summation, the coops2df function will fail if there is no relative water level height above a vertical datum in the observed properties, and the function could also return misleading error messages if an OWSLib exception occurred. The former could be fixed by a simple check for whether the relative water surface height is present in the dataframe column names, returning the dataframe if present, and an empty dataframe if not present.

ocefpaf commented 10 years ago

I will try to clarify some of the issues you raised:

PS: @benjwadams Can you make those comment in the code? It makes it is easier to follow.

birdage commented 10 years ago

@benjwadams @ocefpaf i think this might be related to https://github.com/ioos/system-test/issues/100

ocefpaf commented 10 years ago

Yes @birdage it is. We have to deal with owslib.ows.ExceptionReport. My approach is to catch the exception and list the "offending" station (cell 10). That is not ideal, but it is a work around until we find a way to get/convert the SSH to a desired datum.

@benjwadams That is one reason why I prefer to deal with the exception outside the function. Returning an empty dataframe hides the information of the exception from the users and might give the wrong idea that there is no data.