Skip to content
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

First try at changing the logging facilities #544

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

Hugovdberg
Copy link

@Hugovdberg Hugovdberg commented Oct 12, 2024

Ages ago I proposed to migrate to the built in logging facilities to allow for more flexible logging (see #315). Recently I have been using pyemu again and faced the same issue I faced then so here is a PR (work in progress) to add logging instead of writing files directly.
I decided to first tackle the new run_sp method as this was the newest so least likely to break peoples workflows should it be not as stable as I think it is (see #539 / #532).

What I have done is create a little method that returns a logging.Logger object given the boolean flags verbose and logger (which currently is called logfile in other methods). However, logger can also accept a logging.Logger object directly. So if you run something like the following code, all messages are recorded in my_custom.log instead of pyemu.log:

I have added two helpers, one is pyemu.log_utils.get_logger that returns a logging.Logger object that writes to pyemu.log or stdout when requested. The second is pyemu.log_utils.set_logger that either takes a logging.Logger (as a positional only argument) or keyword arguments that are passed directly to logging.basicConfig:

import logging
import pyemu

# option 1: no config, defaults will be used when messages are logged
# Nothing to show here

# option 2: pass basic config arguments to set_logger
pyemu.log_utils.set_logger(filename="my_custom.log", format="{asctime} :: {levelname} :: {message}", style="{")

# option 3: configure your own Logger and pass that to set_logger
logger = logging.Logger()
file_handler = logging.FileHandler("pyemu.log")
file_handler.setFormatter(logging.Formatter("{asctime} :: {name} :: {levelname} :: {message}", style="{"))
logger.addHandler(file_handler)
stdout_handler = logging.StreamHandler(sys.stdout)
stdout_handler.setFormatter(logging.Formatter("{asctime} :: {levelname} :: {message}", style="{"))
logger.addHandler(stdout_handler)
pyemu.log_utils.set_logger(logger)
pyemu.os_utils.run("pestchek case.pst", use_sp=True, logfile=logger)

Of course you can go all out in how complicated you want your logging facilities to be, see https://docs.python.org/3/library/logging.html# for all possibilities.

@p-ortega
Copy link
Collaborator

p-ortega commented Oct 14, 2024

Hey @Hugovdberg, I think you would have to modify or remove this test, utils_test.py::run_sp_capture_output_test, for the CI workflow to work with the new logging capabilities.

Sorry that test was pretty ad-hoc

@Hugovdberg
Copy link
Author

Probably a lot of tests will fail once we add this in other places as well :-) testing output is always troublesome..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants