-
Notifications
You must be signed in to change notification settings - Fork 0
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
34 add propper logging #36
Changes from all commits
66b2654
f5185ce
f1f07bc
2ad719f
7ef252d
3e9891d
f431418
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,56 @@ | ||||||
import logging | ||||||
import sys | ||||||
import time | ||||||
from contextlib import contextmanager | ||||||
from pathlib import Path | ||||||
from logging import handlers | ||||||
|
||||||
import numpy as np | ||||||
import pandas as pd | ||||||
|
||||||
|
||||||
def setup_logger(name: str = None, stdout_level=logging.INFO): | ||||||
""" | ||||||
Setup a logger with a specific name and logging level. | ||||||
""" | ||||||
|
||||||
path = Path('log') | ||||||
path.mkdir(exist_ok=True, parents=True) | ||||||
|
||||||
formatter = logging.Formatter("%(asctime)s - %(name)s - %(levelname)s - %(message)s") | ||||||
|
||||||
# Create handlers | ||||||
debug_fh = logging.handlers.RotatingFileHandler(path / 'debug.log', maxBytes=10 * 1024 * 1024, backupCount=5) | ||||||
debug_fh.setLevel(logging.DEBUG) | ||||||
debug_fh.setFormatter(formatter) | ||||||
info_fh = logging.handlers.RotatingFileHandler(path / 'info.log') | ||||||
info_fh.setLevel(logging.INFO) | ||||||
info_fh.setFormatter(formatter) | ||||||
stdout_sh = logging.StreamHandler(sys.stdout) | ||||||
stdout_sh.setLevel(stdout_level) | ||||||
stdout_sh.setFormatter(formatter) | ||||||
|
||||||
# Configure logger and add handlers | ||||||
root_logger = logging.getLogger() | ||||||
root_logger.setLevel(logging.DEBUG) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Following the suggestion on
Suggested change
|
||||||
root_logger.addHandler(debug_fh) | ||||||
root_logger.addHandler(info_fh) | ||||||
root_logger.addHandler(stdout_sh) | ||||||
|
||||||
return root_logger | ||||||
|
||||||
|
||||||
@contextmanager | ||||||
def log_duration(task_name: str, logger: logging.Logger): | ||||||
""" | ||||||
Log duration of a task. | ||||||
""" | ||||||
logger.info(f"{task_name} started") | ||||||
start_time = time.perf_counter() | ||||||
yield | ||||||
duration = time.perf_counter() - start_time | ||||||
logger.info(f"{task_name} finished in {duration:.4f} seconds") | ||||||
|
||||||
|
||||||
def log_array(data: np.ndarray, logger, array_name:str="array") -> None: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this function, despite the level of Maybe we should consider check the level of IMO maybe it's better to move the calculation part out of this function and pass them as a dictionary in here. I think when calling a logging function, the computation effort is usally neglected. The user should be aware what calculation is actually done. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point! I was focusing on gathering all data that we'd need to debug a user's problem, I didn't consider performance enough. |
||||||
logger.debug(f"{array_name} shape: {data.shape}") |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
import logging | ||
from pathlib import Path | ||
|
||
from PIL import Image | ||
|
@@ -19,7 +20,7 @@ def image_path_to_mask_path(image_path: Path) -> Path: | |
self.masks = [str(image_path_to_mask_path(Path(p))) for p in self.images][:self.limit] | ||
non_existing_masks = [p for p in self.masks if Path(p).exists() == False] | ||
if non_existing_masks: | ||
print(f"{len(non_existing_masks)} of a total of {len(self.masks)} masks not found.") | ||
logging.getLogger().warning(f"{len(non_existing_masks)} of a total of {len(self.masks)} masks not found.") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. depends on how we would like this function envolve, maybe consider initiate a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good catch! thanks! |
||
|
||
if self.limit is None: | ||
self.limit = len(self.images) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider expose the path of log file as an option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about that, but I think that adds unnecessary complexity for now. If you think we need it (now or soon), especially from the plugin side, I'll go for it of course.