search

CI-WATER / gsshapy

An SQLAlchemy ORM for GSSHA model files and a toolkit to convert gridded input into GSSHA input.
BSD 3-Clause "New" or "Revised" License
7 stars 9 forks source link

Consider making it more obvious that gssha is required #29

Closed ceball closed 7 years ago

ceball commented 7 years ago

I have two questions, both related to confusion newcomers might experience when starting with gsshapy. I'm asking the two questions together in case there's an underlying design reason for both.

First (unless I have missed it!), the documentation doesn't seem to mention that installing gssha is required. Would a PR on http://gsshapy.readthedocs.io/en/latest/intro.html#installation adding something along the lines of the following be any good?

Installation

First, you will need to download and install GSSHA, and then ensure that the gssha executable is on your PATH.

Second, it seems like failure to find the gssha binary should result in an obvious, early exception rather than the exception being caught and only logged as a warning as happens at present. Particularly when (according to the docs)

GsshaPy uses the default Python logging module [and by] default, nothing is logged anywhere.

In general it seems like quite a lot of exceptions are caught and logged throughout gsshapy. It's great to see a project taking logging seriously because that can be very useful. However, as a user I'd also like to find out immediately about things like missing the simulator or a critical file required for the run.

jbednar commented 7 years ago

If there's a reason that raising exceptions is problematic for some use cases (e.g. when embedded in a workflow tool), then it would be good if suppressing those exceptions were enabled by a specific flag when gssahpy is invoked, and then the workflow tool can invoke that flag ("exceptions_as_warnings", perhaps) while still raising exceptions for other types of uses.

snowman2 commented 7 years ago

@ceball @jbednar, you are correct. That should be more obvious. It is not as yet because of testing and my lack of making it more user friendly. I don't actually use the GSSHA executable in testing as there is not an official version for OSX and Linux to test and so I just skip over that part and test it manually.

@ceball, if you would like to make the proposed PR, that would be fantastic. However, it should be noted that it is optional as only parts of the code use GSSHA.

snowman2 commented 7 years ago

@ceball, this is the file to update: https://github.com/CI-WATER/gsshapy/blob/master/docs/intro.rst