-
Notifications
You must be signed in to change notification settings - Fork 62
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
Gather and expose errors #453
Conversation
3bba6b0
to
0e31c7c
Compare
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.
Looks great 👍👍
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.
PR Overview
This PR gathers and exposes various error classes by centralizing exception definitions and providing descriptive docstrings to aid users in understanding the error context. Key changes include:
- Introducing the new module gokart/errors/init.py that aliases several error classes.
- Adding descriptive docstrings to existing exceptions in gokart/pandas_type_config.py, gokart/build.py, and gokart/task.py.
- Updating gokart/slack/init.py to include new error types along with existing components.
Reviewed Changes
File | Description |
---|---|
gokart/errors/init.py | Created to re-export error classes from various modules. |
gokart/slack/init.py | Updated to import and expose new Slack-related error types. |
gokart/pandas_type_config.py | Added a docstring to the PandasTypeError for clarity. |
gokart/build.py | Added a docstring for GokartBuildError and HasLockedTaskException; note a potential typo in parameter naming. |
gokart/task.py | Updated the EmptyDumpError docstring to improve clarity. |
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
gokart/build.py:44
- Typo in parameter name 'messsage'. It should be corrected to 'message' to ensure clarity and consistency.
def __init__(self, messsage, raised_exceptions: dict[str, list[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.
LGTM! Thank you!
Thank you for reviewers! |
@mamo3gr Merged! Thank you for contribution! |
Motivation
Gokart users catch some errors to deal with them according to the errors' details in their applications. But exceptions of this library exist (and will be created in the future) in each sub-module. Users would need to locate their desired exception and keep it in mind.
Proposal
Errors should be gathered and exposed in
gokart.errors
module.Descriptions would be helpful for the users.
Discussion
Moving actual implementations or aliasing
By now, I think it is sufficient to just alias the exceptions (import them from
gokart.errors
module) because existing exceptions are raised from a single source in many cases, and they are located near the source (raiser).Location to gather
As an alternative way, exceptions can be located under
gokart
module. In that casegokart.__init__.py
would be a little messy.