bird-house / flyingpigeon

WPS processes for climate model data, indices and extreme events
http://flyingpigeon.readthedocs.io/en/latest/
Apache License 2.0
19 stars 15 forks source link

[WIP] Consistency in error handling #255

Closed Zeitsperre closed 6 years ago

Zeitsperre commented 6 years ago

For the rationale, see #254

I'd like to implement better/consistent error handling across all algorithms. Any advice on best practices and leveraging Python 2/3 compatible approaches is welcome.

cehbrecht commented 6 years ago

@Zeitsperre I'm also in favor of the new format style and I try to get used to it :) If you like to change it, would be nice. But I won't be to strict on old-style ;)

In this PR you are fixing the exception catching, which pep8 complains about (maybe also necessary for Python 3.x?): except Exception as e:

There is much more to clean up. Probably it is easier to start with a single process.

Zeitsperre commented 6 years ago

Hey @cehbrecht, all good on the newer style string formatting. I think the reason I've been adamant about using the old style is that most of the major libraries and documents on error-handling I've looked at use it, but these libraries are old. I'll refactor to the newer standard.

The undefined Exception is considered bad practice / not PEP8 because I'm not specifying what to look out for beyond base user errors. What I need to know is what types of errors would most likely happen (FileError, IOError, ValueError, TypeError, etc.). What I could do is make exception clauses for user-based errors and add a bare except for really nasty system errors. In any case, I'll continue running through them while not breaking the API.

bekozi commented 6 years ago

Does birdhouse implement its own specialized exceptions? Like:

from abc import ABCMeta
class AbstractBirdhouseException(Exception, metaclass=ABCMeta):
    def __init__(self, message=None):
        self.message = message
cehbrecht commented 6 years ago

@bekozi We did also define our own exceptions ... and another thing to clean up ;) https://github.com/bird-house/flyingpigeon/blob/de9f1f7b2ec6b7be236c01b71364b76a6cc44644/flyingpigeon/exceptions.py#L1

@Zeitsperre If we have some common self-defined exceptions they might also be a candidate for eggshell.

bekozi commented 6 years ago

Thanks @cehbrecht. Sorry to bust in!

Zeitsperre commented 6 years ago

All good. It's giving me something new to look into! @bekozi How would I go about implementing something with specific checks? I imagine I'd look for conditions such as whether the end results are readable in xarray/ocgis. Any suggestions you have for going forward would be good to know.

bekozi commented 6 years ago

How would I go about implementing something with specific checks?

@Zeitsperre This is a funky example with decorators and subclassed exceptions: https://gist.github.com/bekozi/a66a22bf8f06cd914f1f7c7f88e9077f. You could accomplish something similar with objects instead of functions I think. The trick is making sure you get the full traceback in the log which I didn't really look into...