-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
feat(python): Standardize error message format #11598
feat(python): Standardize error message format #11598
Conversation
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.
A lot of improvements here!
I also spotted quite some examples which have become less clear. Please take a look at some of my comments and go through the changes again to double check. Then I'll do a final pass and we can merge this.
py-polars/polars/dataframe/frame.py
Outdated
@@ -3436,14 +3436,15 @@ def write_database( | |||
elif engine == "sqlalchemy": | |||
if parse_version(pd.__version__) < parse_version("1.5"): | |||
raise ModuleNotFoundError( | |||
f"writing with engine 'sqlalchemy' requires pandas 1.5.x or higher, found pandas {pd.__version__!r}" | |||
f"writing with engine `sqlalchemy` requires pandas 1.5.x or higher, found pandas {pd.__version__!r}" |
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.
sqlalchemy isn't a code snippet - it's a name of a package. It shouldn't get backticks.
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.
@stinodego Understood. Whilst editing I saw that packages are sometimes within single quotes, sometimes with CamelCase notation. Any standardization you would like there as well, e.g. always in single quotes and all lowercase?
cbd2a97
to
583343a
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.
Thanks! I made an additional pass to clean up some things, and this can be merged.
Addresses #10421 for
py-polars
folder but not rust yet. Some unit tests had to be updated as well due to the new formatting.All files in
py-polars
have been searched onraise
and have been checked on the bullet points mentioned in #10421.