g-battaglia / kerykeion

Data driven Astrology 💫
Kerykeion is a python library for astrology. It can generate SVG charts and extract all data about a birthchart, a synastry chart and a transit chart.
https://kerykeion.net
GNU Affero General Public License v3.0
297 stars 103 forks source link

Use of logging.basicConfig() throughout disrespects user settings #85

Closed E-werd closed 1 year ago

E-werd commented 1 year ago

I use logging in my own project with a specific format declared at the beginning. When I import and use this library that format is altered to use what Kerykeion uses.

Suggestion:

It appears that you're doing this so that you can test each individual file on its own, hence if __name__ == "__main__: throughout. You could move basicConfig() into that if block and achieve the same goal, respecting the settings that the users of the library prefer.

Thanks!

g-battaglia commented 1 year ago

Thank you very much for the suggestion! I've tried to figure out the best way to implement logging across the library, but I didn't actually find it.

In your opinion, what would be the best way to create the logger in the file? Could you please open a pull request (even if it's just for one file) with the way you would implement it?

Or even better, if you know of another package/library that implements the logger in a better way, could you suggest it to me?

There's a lot to refactor across the library, and I'm trying to do it little by little, but I think most of it will be in the version 5 release. Any help is super useful.

E-werd commented 1 year ago

Or even better, if you know of another package/library that implements the logger in a better way, could you suggest it to me?

There's no problem with the logging library, it's a fine and common choice.

In your opinion, what would be the best way to create the logger in the file? Could you please open a pull request (even if it's just for one file) with the way you would implement it?

Pull request #86 submitted with relevant changes. I tested every file where I made changes, all worked fine and functionally identical to the previous implementation.

g-battaglia commented 1 year ago

Great, thank you very much! Merged and released in version 4.2.4!