From debdc758486791a9cf0e474594021a85c594dfff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Antoine=20Goudreault?= Date: Sat, 2 Dec 2017 22:38:48 -0500 Subject: [PATCH] Fixed logging problems and inconsistensies - Got rid of the warnings module by putting a logger into the sheet managers. - loglevel is better passed to the excel manager and sheet managers. --- auxiclean/managers/config_manager.py | 2 +- auxiclean/managers/excel_manager.py | 29 +++++++++++++---------- auxiclean/managers/sheet_managers.py | 19 +++++++++++---- auxiclean/selector.py | 2 +- auxiclean/unittests/test_excel_manager.py | 10 +------- 5 files changed, 34 insertions(+), 28 deletions(-) diff --git a/auxiclean/managers/config_manager.py b/auxiclean/managers/config_manager.py index 93432fd..7020e0b 100644 --- a/auxiclean/managers/config_manager.py +++ b/auxiclean/managers/config_manager.py @@ -35,7 +35,7 @@ class MissingPriority(Exception): class ConfigManager: def __init__(self, path=DEFAULT_CONFIG_PATH, loglevel=logging.INFO): logging.basicConfig() - self._logger = logging.getLogger("ConfigManager") + self._logger = logging.getLogger("auxiclean.managers.config_manager") self._logger.setLevel(loglevel) self._config = configparser.ConfigParser() diff --git a/auxiclean/managers/excel_manager.py b/auxiclean/managers/excel_manager.py index 95530c7..7f8d511 100644 --- a/auxiclean/managers/excel_manager.py +++ b/auxiclean/managers/excel_manager.py @@ -7,13 +7,14 @@ class ExcelFileManager: def __init__(self, path, loglevel=logging.INFO): - self.logger = logging.getLogger("auxiclean.managers.excelfilemanager") - self.logger.setLevel(loglevel) + logging.basicConfig() + self._logger = logging.getLogger("auxiclean.managers.excelfilemanager") + self._logger.setLevel(loglevel) self.path = path self.wb = load_workbook(path) - self._courses = CoursesSheetManager(self.wb) - self._candidates = CandidatesSheetManager(self.wb) + self._courses = CoursesSheetManager(self.wb, loglevel=loglevel) + self._candidates = CandidatesSheetManager(self.wb, loglevel=loglevel) self._checkup() self._distribution = None @@ -27,7 +28,9 @@ def candidates(self): def write_distribution(self, distribution): if self._distribution is None: - self._distribution = DistributionSheetManager(self.wb) + level = self._logger.level + self._distribution = DistributionSheetManager(self.wb, + loglevel=level) self._distribution.write_distribution(distribution) self.save() @@ -68,19 +71,19 @@ def _checkup(self): if d is None or d.lower() == "générale": continue if d not in all_courses_disciplines: - self.logger.warning("La discipline '%s' de %s n'est pas dans" - " la liste de toutes les" - " disciplines des cours." % - (d, candidate.name)) + self._logger.warning("La discipline '%s' de %s n'est pas dans" + " la liste de toutes les" + " disciplines des cours." % + (d, candidate.name)) for course in self.courses: d = course.discipline.lower() if d is None or d.lower() == "générale": continue if d not in all_candidates_disciplines: - self.logger.warning("La discipline '%s' du cours %s n'est" - " pas dans" - " la liste de toutes les disciplines des" - " candidatures." % (d, course.code)) + self._logger.warning("La discipline '%s' du cours %s n'est" + " pas dans" + " la liste de toutes les disciplines des" + " candidatures." % (d, course.code)) def _choice_in_courses(self, choice): for course in self.courses: diff --git a/auxiclean/managers/sheet_managers.py b/auxiclean/managers/sheet_managers.py index e3ce955..af1d256 100644 --- a/auxiclean/managers/sheet_managers.py +++ b/auxiclean/managers/sheet_managers.py @@ -2,12 +2,17 @@ from ..course import Course from ..exceptions import ExcelError from openpyxl.styles import Alignment, colors, PatternFill, Font -import warnings +import logging class BaseSheetManager: - def __init__(self, workbook): + _loggername = None + + def __init__(self, workbook, loglevel=logging.INFO): self.wb = workbook + logging.basicConfig() + self._logger = logging.getLogger(self._loggername) + self._logger.setLevel(loglevel) def _find_max_columns(self, titles_row): for i, cell in enumerate(titles_row): @@ -25,6 +30,8 @@ def get_sheet(self, workbook, sheetname): class CoursesSheetManager(BaseSheetManager): + _loggername = "auxiclean.managers.courses_sheet_manager" + def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self._courses = None @@ -57,6 +64,8 @@ def load_courses_from_wb(self, wb): class CandidatesSheetManager(BaseSheetManager): + _loggername = "auxiclean.managers.candidate_sheet_manager" + def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self._candidates = None @@ -115,6 +124,7 @@ def _get_list_from_str(self, string): class DistributionSheetManager(BaseSheetManager): + _loggername = "auxiclean.managers.distribution_sheet_manager" def write_distribution(self, distribution): # get sheet @@ -174,6 +184,7 @@ def get_sheet(self, workbook): else: # sheet exists, warn user, but create one ws = workbook.create_sheet("Distribution") - warnings.warn("A Distribution sheet already exists in destination." - " A new one will be created: %s." % ws.title) + self._logger.warning("A Distribution sheet already exists" + " in destination." + " A new one will be created: %s." % ws.title) return ws diff --git a/auxiclean/selector.py b/auxiclean/selector.py index 63e2ce5..88f8c57 100644 --- a/auxiclean/selector.py +++ b/auxiclean/selector.py @@ -29,7 +29,7 @@ def __init__(self, path, master=None, self.logger.setLevel(loglevel) self.master = master # if in a GUI # load courses and candidates from excel file - self.excel_mgr = ExcelFileManager(path) + self.excel_mgr = ExcelFileManager(path, loglevel=loglevel) self.courses = self.excel_mgr.courses self.candidates = self.excel_mgr.candidates # print courses and candidates diff --git a/auxiclean/unittests/test_excel_manager.py b/auxiclean/unittests/test_excel_manager.py index 359cb44..1476a46 100644 --- a/auxiclean/unittests/test_excel_manager.py +++ b/auxiclean/unittests/test_excel_manager.py @@ -3,7 +3,6 @@ from auxiclean import Selector from auxiclean.exceptions import ExcelError from collections import OrderedDict -import warnings class TestExcelManager(TestBase): @@ -65,14 +64,7 @@ def test_distribution_sheet_already_exists(self): wb.save(self.data_path) del wb # call selector and check that already existing sheet is not erased - # also check that a user warning is raised - with warnings.catch_warnings(record=True) as w: - warnings.simplefilter("always") - self.selector = Selector(self.data_path) - # check a warning is raised (only one) - self.assertEqual(len(w), 1) - self.assertIn("Distribution sheet already exists", - str(w[-1].message)) + self.selector = Selector(self.data_path) # reload wb wb = load_workbook(self.data_path) self.assertIn("Distribution1", wb.sheetnames)