OpenWaterAnalytics / epanet-python

python wrapper for epanet library
76 stars 40 forks source link

exceptions are thrown but I still want the return value #46

Closed samhatchett closed 5 years ago

samhatchett commented 5 years ago

when iterating through a hydraulic simulation using the nextH-type methods, if there is an epanet-related error like "negative pressures", the stepping call will raise an exception. We can catch it, but we will not get the time return value from the function (which we need for various reasons).

I see that the toolkit.i file declares some macros to accomplish both routing of OUTVALUEs to returns, and checking the C toolkit return value to raise, but this is preventing us from using the functions as we desire.

our workaround seems to be changing the swig interface file to avoid raising, or to avoid passing the OUTVALUE as a return so we can retrieve it even if the function raises.

any thoughts?

michaeltryby commented 5 years ago

@samhatchett Like you say, we can ...

  1. Not throw exceptions for a particular function. Very easy to do, not very satisfying as it doesn't really solve the problem (maybe a temporary workaround).
  2. Set-up the offending function to return it's error code. Not satisfying as it's not very Pythonic.

Or ...

  1. Wire up the python wrapper to give more specific warnings instead of the general warnings. And have the return values passed out in the message body when the exception gets thrown. Maybe do a custom exception class for warnings so you can catch them strip out the return value and keep going.
michaeltryby commented 5 years ago

Another thought ...

  1. Only throw exceptions for unrecoverable errors and not for warnings.
samhatchett commented 5 years ago

I think my leaning is to not throw exceptions for non-exceptional behavior. Exceptions (at least in the C-like world) are for crazy things that should not happen but might on occasion mess something up.

In the JavaScript world, this would be a situation for passing an error callback to the function in question - which is very javascripty.

I'm not so clear on what is pythonic - but it looks like exceptions are pretty ubiquitous since you can also do crazy (i.e. "pythonic") things like:

When an exception occurs, it may have an associated value, also known as the exception’s argument. The presence and type of the argument depend on the exception type.

... which may be the way to go

michaeltryby commented 5 years ago

Looks like they already thought of this ... https://docs.python.org/3/library/warnings.html

michaeltryby commented 5 years ago

@samhatchett this turned out to be fairly straight forward. Since err_check() was already returning the error code it was available to the wrapper. All I needed to do was modify the custom exception logic in toolkit.i. I wrote some tests and we should be in business.

samhatchett commented 5 years ago

Wow that’s incredible. Thanks Michael!

michaeltryby commented 5 years ago

Updated wheels can be found here:

https://ci.appveyor.com/project/michaeltryby/epanet-python/branch/dev/artifacts