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

Standardize and expand logging within gempyor #310

Open
emprzy opened this issue Sep 11, 2024 · 2 comments
Open

Standardize and expand logging within gempyor #310

emprzy opened this issue Sep 11, 2024 · 2 comments
Assignees
Labels
enhancement Request for improvement or addition of new feature(s). gempyor Concerns the Python core. low priority Low priority.

Comments

@emprzy
Copy link
Collaborator

emprzy commented Sep 11, 2024

Label

enhancement, post-processing

Priority Label

low priority

Is your feature request related to a problem? Please describe.

As we continue to evolve flepiMoP and move towards better practices (particularly as it relates to improving the readability/consistency of exceptions and errors; noted in issue #281), we should work to remove print() statements called during errors.

Is your feature request related to a new application, scenario round, pathogen? Please describe.

No response

Describe the solution you'd like

A more ideal way to display extensive messages related to errors or warnings is to use a logger. There are several files in gempyor that already have loggers, but still there are exceptions or errors that contain print() statements. We should:

  1. Ensure that every file is using a logger (either configured at the file level or at the module level; depends on y'all's preference)
  2. Ensure that the logger is a replacement for poor practice print() statements, not an additive
@github-actions github-actions bot added enhancement Request for improvement or addition of new feature(s). gempyor Concerns the Python core. low priority Low priority. post-processing Concern the post-processing. labels Sep 11, 2024
@TimothyWillard TimothyWillard removed the post-processing Concern the post-processing. label Sep 12, 2024
@TimothyWillard
Copy link
Contributor

Some brief thoughts I have on this:

  1. I think creating a factory for creating a logger instance with our preferred settings will life easier for us and updating logging settings/features in the future painless. Say create_logger in a new gempyor.logging that does something like:
def create_logger(name, format="%(asctime)s:%(name)s:%(levelname)s:%(message)s", log_level=logging.WARNING):
  logger = logging.getLogger(name)
  stream_handler = logging.StreamHandler()
  stream_handler.setLevel(log_level)
  log_formatter = logging.Formatter(format)
  stream_handler.setFormatter(log_formatter)
  logger.addHandler(stream_handler)
  return logger
  1. We have to give some thought as to how to pass logging settings. I think the three clear paths we have are a) pull logging settings from a CLI interface, which requires passing around variables and having consistent CLIs, b) pull logging settings from environment variables, a bit clunky from a user prospective and limits the types of settings that can be passed, c) pull logging settings from the config yaml files, but if there's an issue with the config yaml file it probably won't be caught by logging in this case.

The python documentation on the logging module is quite detailed: https://docs.python.org/3/howto/logging.html. I'm also open to other libraries with the caveats that they have to be stable, well maintained, and provide significant value over the stdlib. These are also brief preliminary thoughts, I'm open to different solutions/approaches entirely as well.

I think this is behind GH-281, is that the right priority?

@emprzy
Copy link
Collaborator Author

emprzy commented Sep 13, 2024

I agree with what you said about creating a logging factory – I didn't previously know that terminology but I definitely think we should do our best to universalize our logging conventions, so implementing at a more "upstream" logger would be a good way to do this. As for the minutiae of logging settings, I don't have much of an opinion as a product of never having worked with a logger before. But, as I look more into it, I may develop some opinions. Open to any and all direction!
And yep, this needs to be subsequent to issue #281 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Request for improvement or addition of new feature(s). gempyor Concerns the Python core. low priority Low priority.
Projects
None yet
Development

No branches or pull requests

2 participants