mir_eval icon indicating copy to clipboard operation
mir_eval copied to clipboard

Exception classes?

Open bmcfee opened this issue 8 years ago • 3 comments

In implementing #242 , I noticed that the chord submodule defines a derived exception class, but it's the only place where we do this. Everywhere else, primitive exception types (valueerror, typeerror, etc) are used.

This isn't good practice because it makes it difficult for an end-user to encapsulate exceptions based on what level of the call stack they come from.

Instead, we should have a base exception class MIRException from which we can derive more specific exception classes in a coherent hierarchy. That way, a user can catch any MIRException on parse/eval without having to worry about all the various types of ways that an eval can go wrong.

What do yall think?

bmcfee avatar Mar 17 '17 18:03 bmcfee

I'm not opposed to this at all, but I don't think we have intricate enough exceptions to warrant this, by which I mean there aren't many functions (OTOH, would be happy to be corrected) that raise enough different exceptions that the end-user would want to handle separately.

craffel avatar Mar 17 '17 18:03 craffel

It's less about having a deep/wide hierarchy of exceptions than making it easy to catch all mir_eval-generated exceptions in one go, if the caller wants to. Right now, we can't distinguish mir_eval errors from numpy errors, or anything else.

bmcfee avatar Mar 17 '17 18:03 bmcfee

My point (as usual) is basically that this is worth implementing once people start complaining because they don't have enough fine-grained exception control when they use mir_eval, and that is stopping them from doing what they want to.

craffel avatar Mar 17 '17 19:03 craffel