Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception classes? #243

Open
bmcfee opened this issue Mar 17, 2017 · 3 comments
Open

Exception classes? #243

bmcfee opened this issue Mar 17, 2017 · 3 comments

Comments

@bmcfee
Copy link
Collaborator

bmcfee commented Mar 17, 2017

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?

@craffel
Copy link
Collaborator

craffel commented Mar 17, 2017

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.

@bmcfee
Copy link
Collaborator Author

bmcfee commented Mar 17, 2017

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.

@craffel
Copy link
Collaborator

craffel commented Mar 17, 2017

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.

@bmcfee bmcfee added this to the 0.8 milestone Mar 13, 2024
@bmcfee bmcfee removed this from the 0.8 milestone Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants