PaulHancock / Aegean

The Aegean source finding program and associated tools
http://aegeantools.rtfd.io/
Other
47 stars 14 forks source link

Logging to root python logger #202

Open tjgalvin opened 1 year ago

tjgalvin commented 1 year ago

Hi Paul - I noticed some strange logging behavior when attempting to use components from AegeanTools in some other workflows. Specifically, it looks like there is an extra handler being added to the root logger, which in turn is being passed down to other modules:

INFO 2023-08-22 13:19:47.344 aegean - run_bane: Using base output name of: 2023-08-06_182942_2.EMU_0450-41.round1-MFS-image
INFO 2023-08-22 13:19:58.557 aegean - run_bane: Have finished running BANE.
INFO:flint:Have finished running BANE.

I went looking and found a couple components that are writing out through the logging module directly. Something like:

import logging

logging.debug("Some message")

This also is a little confusing as there is also some custom formatter being attached somewhere. I think having a single logging submodule that sets up the aegean logger, which is then imported into other scripts, is the neatest way to get around the pains of the logging module. I have raised #201 as a point of discussion. I have not thoroughly tested it (in fact I haven't), but hopefully it is enough to show off what I mean.

The hope is that this will only make it easier for other python codes to hook into the heart of aegean for their own profit on the high seas.

PaulHancock commented 1 year ago

I never noticed this before, but after looking at some of the changes you made in the PR, it's clear that I wasn't as careful as I should have been. Good work on picking this up.

tjgalvin commented 1 year ago

Thanks for the review. I will try to get to them tomorrow. It really was a set of find and replaces with a quick skim. There are likely other things to fix in the submodule components (CLI for instance).

I only really noticed it as I was trying to use aegean without having to make a sub-process call. Would much rather import the core components directly. Thats when this logger business stuck out.