Skip to content
This repository has been archived by the owner on Apr 11, 2019. It is now read-only.

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

Open
benjwadams opened this issue Aug 12, 2014 · 3 comments
Labels

Comments

@benjwadams
Copy link
Contributor

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
Copy link
Member

ocefpaf commented Aug 12, 2014

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

  • Bare in mind that coops2df is not generic enough to be used out of its scope! (We should try to implement a more generic version though.)
  • The if False statement is a way to escape code, akin to a comment "#", but keeping the syntax highlight active. It is just a coding style and nothing more. The comment was there since coops2df earlier development.
  • I simplified coops2df logic for my specific use case here, and moved the datum check logic outside the function for better readability and error messages. I strongly discourage to implement the check logic you suggested inside the function. Let the function fail and capture the desired exception outside, in the notebook main code, so we can see what is happening, and deal if it properly . (Like in my case, for example, I do not want an empty dataframe returned. I want the station position to report where I have the wrong datum.)

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

@birdage
Copy link
Contributor

birdage commented Aug 13, 2014

@benjwadams @ocefpaf i think this might be related to #100

@ocefpaf
Copy link
Member

ocefpaf commented Aug 13, 2014

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.

@jkupiec jkupiec added the bug label Nov 25, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants