Add logger and replace print calls with appropriate logging functions #59
Conversation
This commit adds a logger.py file and also replaces all `print` calls with logging functions like `logger.info`, `logger.warning`, `logger.error` etc. Also adds rich as a dependency.
Aathish04
left a comment
Aathish04
left a comment
There was a problem hiding this comment.
Works flawlessly.
FOR FUTURE CONSIDERATION:
Implementing custom colour palettes.
leotrs
left a comment
leotrs
left a comment
There was a problem hiding this comment.
I like where this is going. The next step is to add logging messages where there were none. Maybe a separate PR?
| print("Please use a valid color") | ||
| print(err) | ||
| logger.warning("Please use a valid color") | ||
| logger.error(err) |
There was a problem hiding this comment.
These two should be logged at the same level; it looks like it should be error here.
There was a problem hiding this comment.
Or you could be fancy and use logger.exception(). Likewise inside the other exception blocks.
| ## Setting config for logger. | ||
| logging.basicConfig( | ||
| level="NOTSET", | ||
| format="%(message)s", |
There was a problem hiding this comment.
I actually think the filename could come in handy in logs. Maybe something that could be flag-configurable later.
| print("Please use a valid color") | ||
| print(err) | ||
| logger.warning("Please use a valid color") | ||
| logger.error(err) |
There was a problem hiding this comment.
Or you could be fancy and use logger.exception(). Likewise inside the other exception blocks.
|
Just a small thing, Could you do something about the formatting of that? Because it looks ugly next to the great formatting of the logs. EDIT: On second thought I don't know. I'm not an artistic guy, maybe it's better like that |
This commit adds a logger.py file and also replaces all
printcalls with logging functions likelogger.info,logger.warning,logger.erroretc.Also adds rich as a dependency.