Remove json-logging dependency and initialization

[n8downs]

Problem

After updating to 2.0.dev17, we're suddenly seeing our LCA server logs transformed into JSON. While JSON logging is not in and of itself a bad thing, I'd prefer to have control over the format of our logs, rather than have them dictated as a side-effect of importing a dependency package. Prior to 9b2ce5b, it looks like this logging init was optional.

Before

bw-calculator  | Started server process [8]
bw-calculator  | Waiting for application startup.
bw-calculator  | Application startup complete.

After

bw-calculator  | {"written_at": "2024-07-03T09:25:37.116Z", "written_ts": 1719998737116708000, "msg": "Started server process [8]", "type": "log", "logger": "uvicorn.error", "thread": "MainThread", "level": "INFO", "module": "server", "line_no": 82}
bw-calculator  | {"written_at": "2024-07-03T09:25:37.116Z", "written_ts": 1719998737116938000, "msg": "Waiting for application startup.", "type": "log", "logger": "uvicorn.error", "thread": "MainThread", "level": "INFO", "module": "on", "line_no": 48}
bw-calculator  | {"written_at": "2024-07-03T09:25:37.117Z", "written_ts": 1719998737117367000, "msg": "Application startup complete.", "type": "log", "logger": "uvicorn.error", "thread": "MainThread", "level": "INFO", "module": "on", "line_no": 62}

I couldn't find a way to override this change via something like json_logging.init_non_web(enable_json=False) either before or after the bw2calc import, so I'm open to workaround ideas.

Solution

Remove the dependency on json_logging and the initialization of JSON logging. I'd argue that log format should be a decision that the user makes rather than one determined by this package.

[codecov]

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

[cmutel]

Thanks @n8downs. I totally get where you are coming from, and would be frustrated in your shoes as this change was not listed in the changelog.

Logging can be done in many different ways, and is taken personally by some (though not many) of our users; the philosophy of Brightway is definitely to allow people to do what they want.

I am merging this PR. I have been using loguru in recent development, and would appreciate your input on whether a simpler library like this, but one which still allows for other logging patterns would make sense to you. In particular, I would love some pointers on how we can help make it easy for you to use whatever logger you want while still producing nice and easy logs for the 90% of our users who don't even know that logging is a thing. One possibility would be to add logger as in input argument with loguru as a default, but I am open to other suggestions.

@jsvgoncalves FYI

[n8downs]

Wow, thanks for the quick turnaround!

Happy to provide some thoughts on logging, but feel free to take them with a grain of salt coming from a javascript guy getting back into python.

Loguru looks to be pretty slick! They've got a recommendation for logging as a library: https://github.com/Delgan/loguru?tab=readme-ov-file#suitable-for-scripts-and-libraries

Alternatively, you could follow the general python guidelines for logging as a library: https://docs.python.org/3/howto/logging.html#configuring-logging-for-a-library

Then, to enable logs your users would need to

from loguru import logger
logger.enable('brightway')
# OR
import logging
logging.getLogger('brightway').addHandler(...)

Either way, I think a setup note for how to enable the brightway logs in your users' applications would suffice to make sure users can still get at important logged info while allowing more power users control over their standard out.

[cmutel]

Thanks Nathan-

Disabling logs by default will work in some cases but not others. For example, in bw_simapro_csv we use multiple logs to provide both real-time and after the fact feedback to users, and these logs can't be disabled by default while keeping the library usable.

In these cases, I will try a combination of explicit documentation on configuration and removal, and following the expected loguru conventions more closely.

FYI I would like to do the same with bw2calc, and get feedback from you, before making a new release.

[n8downs]

Yeah, I think as long as it can be disabled and if logging is an important avenue of feedback from the library, it's fine to leave it turned on by default.

Happy to give feedback on this update when it's ready.