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

Meaningful exceptions in convolution #14727

Closed
nstarman opened this issue May 1, 2023 · 2 comments · Fixed by #14728
Closed

Meaningful exceptions in convolution #14727

nstarman opened this issue May 1, 2023 · 2 comments · Fixed by #14728

Comments

@nstarman
Copy link
Member

nstarman commented May 1, 2023

What is the problem this feature will solve?

Convolution raises bare Exceptions!

raise Exception("Kernel operation not supported.")
else:
raise Exception("Kernel operation not supported.")

Describe the desired outcome

Have meaningful errors.
Maybe whip up a quick

class KernelException(Exception):
    """An exception for kernel convolution."""

Bare Excepts are a classic anti pattern.

Additional context

Noticed by Ruff in #14724

@nstarman
Copy link
Member Author

nstarman commented May 1, 2023

We should do a followup to update the tests from pytest.raises(Exception) to pytest.raises(KernelArithmeticError)

Also, maybe all the Kernel error classes should have a common base class? Like KernelError

@larrybradley
Copy link
Member

@nstarman I'm working on a PR to clean up various convolution exceptions. I'll also update the tests from #14724.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants