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

Extend user-defined exception from Exception instead of BaseException #53

Merged
merged 1 commit into from
Nov 14, 2020

Conversation

4m1g0
Copy link
Contributor

@4m1g0 4m1g0 commented Nov 14, 2020

I've noticed that the KaitaiStructError extends BaseException instead of Exception. This is contrary to the python official documentation (https://docs.python.org/3/library/exceptions.html):

programmers are encouraged to derive new exceptions from the Exception class or one of its subclasses, and not from BaseException.

exception BaseException: The base class for all built-in exceptions. It is not meant to be directly inherited by user-defined classes (for that, use Exception).

And makes it not possible to catch a decoding exception when using the standard method:

try:
    g = Gif.from_bytes(message_bytes)
except Exception as e:
    print(e)

Is there a specific reason for extending from BaseException that I'm missing?

Thank you very much!

… to follow Python oficial docummentation and make it possible to catch it using a generic Exception
@generalmimon
Copy link
Member

For your own convenience, remember a rule of thumb when making pull requests from your fork repository: never commit directly on master of your fork. Create a topic branch for every PR you create. See https://contribute.jquery.org/commits-and-pull-requests/#never-commit-on-master:

When you're working on a fork, you should always think of your master branch as a "landing place" for upstream changes. You should only ever make your commits to topic branches, and your own commits should only ever end up on master after they've been merged in upstream by a maintainer.

Copy link
Member

@generalmimon generalmimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks!

@generalmimon generalmimon merged commit c7f2b89 into kaitai-io:master Nov 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants