-
Notifications
You must be signed in to change notification settings - Fork 23
fix: Improve typings for errors #213
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
Conversation
|
|
||
| class C2paError(Exception): | ||
| """Exception raised for C2PA errors.""" | ||
| """Exception raised for C2PA errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixing error class hierarchy to leverage typed errors better, especially in the new Reader factory method
| self.message = message | ||
| super().__init__(message) | ||
|
|
||
| class Assertion(Exception): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specific exceptions (all the types nested in the parent class) got un-nested from here so they can inherit from the base C2pa exception and get the C2paException type.
| # Attach exception subclasses to C2paError for backward compatibility | ||
| # Preserves behavior for exception catching like except C2paError.ManifestNotFound, | ||
| # also reduces imports (think of it as an alias of sorts) | ||
| C2paError.Assertion = _C2paAssertion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An we still can use the parent type for general error catching!
| if ': ' in error_str: | ||
| parts = error_str.split(': ', 1) | ||
| else: | ||
| parts = error_str.split(' ', 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the c api is inconsistent here, we should add an issue to fix it so that we always use the colon
Changes in this pull request
Checklist
TO DOitems (or similar) have been entered as GitHub issues and the link to that issue has been included in a comment.