Skip to content

Commit

Permalink
Fixed logging problems and inconsistensies
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
fgoudreault committed Dec 3, 2017
1 parent f849140 commit debdc75
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 28 deletions.
2 changes: 1 addition & 1 deletion auxiclean/managers/config_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
29 changes: 16 additions & 13 deletions auxiclean/managers/excel_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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()

Expand Down Expand Up @@ -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:
Expand Down
19 changes: 15 additions & 4 deletions auxiclean/managers/sheet_managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion auxiclean/selector.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 1 addition & 9 deletions auxiclean/unittests/test_excel_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from auxiclean import Selector
from auxiclean.exceptions import ExcelError
from collections import OrderedDict
import warnings


class TestExcelManager(TestBase):
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit debdc75

Please sign in to comment.