From 38ef7aae6c3be09c4ca054262176df83fc718785 Mon Sep 17 00:00:00 2001 From: Shelly Belsky Date: Mon, 13 Jan 2025 11:48:33 -0500 Subject: [PATCH] ENH: Incorporate code review suggestions Co-authored-by: Jean-Christophe Fillion-Robin --- AutoscoperM/AutoscoperM.py | 20 ++++++------- .../{ErrorUtils.py => Validation.py} | 30 +++++-------------- AutoscoperM/CMakeLists.txt | 2 +- 3 files changed, 19 insertions(+), 33 deletions(-) rename AutoscoperM/AutoscoperMLib/{ErrorUtils.py => Validation.py} (75%) diff --git a/AutoscoperM/AutoscoperM.py b/AutoscoperM/AutoscoperM.py index d432211..b9e80f3 100644 --- a/AutoscoperM/AutoscoperM.py +++ b/AutoscoperM/AutoscoperM.py @@ -18,7 +18,7 @@ ) from slicer.util import VTKObservationMixin -from AutoscoperMLib import IO, ErrorUtils, SubVolumeExtraction +from AutoscoperMLib import IO, SubVolumeExtraction, Validation # # AutoscoperM @@ -448,7 +448,7 @@ def onGeneratePartialVolumes(self): modelSubDir = self.ui.modelSubDir.text segmentationNode = self.ui.pv_SegNodeComboBox.currentNode() - ErrorUtils.validateInputs( + Validation.validateInputs( volumeNode=volumeNode, segmentationNode=segmentationNode, mainOutputDir=mainOutputDir, @@ -502,7 +502,7 @@ def onGenerateConfig(self): camCalList = self.ui.camCalList # Validate the inputs - ErrorUtils.validateInputs( + Validation.validateInputs( volumeNode=volumeNode, mainOutputDir=mainOutputDir, configFileName=configFileName, @@ -513,7 +513,7 @@ def onGenerateConfig(self): partialVolumeList=partialVolumeList, camCalList=camCalList, ) - ErrorUtils.validatePaths( + Validation.validatePaths( mainOutputDir=mainOutputDir, tiffDir=os.path.join(mainOutputDir, tiffSubDir), radiographSubDir=os.path.join(mainOutputDir, radiographSubDir), @@ -596,7 +596,7 @@ def get_checked_items(listWidget): ] # Validate the extracted parameters - ErrorUtils.validateInputs( + Validation.validateInputs( *trialDirs, *partialVolumeFiles, *camCalFiles, @@ -633,12 +633,12 @@ def onImportModels(self): volumeNode = self.ui.volumeSelector.currentNode() - ErrorUtils.validateInputs(volumeNode=volumeNode) + Validation.validateInputs(volumeNode=volumeNode) if self.ui.segSTL_loadRadioButton.isChecked(): segmentationFileDir = self.ui.segSTL_modelsDir.currentPath - ErrorUtils.validatePaths(segmentationFileDir=segmentationFileDir) + Validation.validatePaths(segmentationFileDir=segmentationFileDir) segmentationFiles = glob.glob(os.path.join(segmentationFileDir, "*.*")) segmentationNode = slicer.mrmlScene.AddNewNodeByClass("vtkMRMLSegmentationNode") @@ -666,7 +666,7 @@ def onSegmentation(self): volumeNode = self.ui.volumeSelector.currentNode() - ErrorUtils.validateInputs(volumeNode=volumeNode) + Validation.validateInputs(volumeNode=volumeNode) if self.ui.segGen_autoRadioButton.isChecked(): currentVolumeNode = volumeNode @@ -791,14 +791,14 @@ def populateListFromOutputSubDir(self, listWidget, fileSubDir, itemType="file"): listWidget.clear() mainOutputDir = self.ui.mainOutputSelector.currentPath - ErrorUtils.validateInputs( + Validation.validateInputs( listWidget=listWidget, mainOutputDir=mainOutputDir, fileSubDir=fileSubDir, ) fileDir = os.path.join(mainOutputDir, fileSubDir) - ErrorUtils.validatePaths(fileDir=fileDir) + Validation.validatePaths(fileDir=fileDir) if itemType == "file": listFiles = [f.name for f in os.scandir(fileDir) if os.path.isfile(f)] diff --git a/AutoscoperM/AutoscoperMLib/ErrorUtils.py b/AutoscoperM/AutoscoperMLib/Validation.py similarity index 75% rename from AutoscoperM/AutoscoperMLib/ErrorUtils.py rename to AutoscoperM/AutoscoperMLib/Validation.py index 2a4bb84..1de2a35 100644 --- a/AutoscoperM/AutoscoperMLib/ErrorUtils.py +++ b/AutoscoperM/AutoscoperMLib/Validation.py @@ -1,10 +1,9 @@ import os -# -# ValueErrorsException: A custom exception class that accepts a list of errors -# class ValueErrorsException(Exception): + """A custom exception class that accepts a list of errors.""" + def __init__(self, errors): if not isinstance(errors, list): raise ValueError("The errors input must be a list") @@ -14,25 +13,18 @@ def __init__(self, errors): super().__init__("\n".join(errors)) def __str__(self): - err_str = "Invalid value" if len(self.errors) == 1 else "Multiple invalid values" + err_str = "Invalid value{}".format("" if len(self.errors) == 0 else "s") return err_str + ":\n" + "\n".join(self.errors) -# -# helper functions -# - - -def validateInputs(*args: tuple, **kwargs: dict) -> bool: +def validateInputs(*args: tuple, **kwargs: dict) -> None: """ Validates that the provided inputs are not None or empty. :param args: list of inputs to validate - :param kwargs: list of inputs to validate + :param kwargs: dictionary of inputs to validate :raises: ValueErrorsException - - :return: True if all inputs are valid """ errors = [] for arg in args: @@ -47,21 +39,17 @@ def validateInputs(*args: tuple, **kwargs: dict) -> bool: if isinstance(arg, str) and arg == "": errors.append(f"Input '{name}' is an empty string") - if len(errors) != 0: + if len(errors) > 0: raise ValueErrorsException(errors) - return True - -def validatePaths(*args: tuple, **kwargs: dict) -> bool: +def validatePaths(*args: tuple, **kwargs: dict) -> None: """ Checks that the provided paths exist. :param args: list of paths to validate :param kwargs: list of paths to validate :raises: ValueErrorsException - - :return: True if all paths exist """ errors = [] for arg in args: @@ -72,7 +60,5 @@ def validatePaths(*args: tuple, **kwargs: dict) -> bool: if not os.path.exists(path): errors.append(f"Input path '{name}' ({path}) does not exist") - if len(errors) != 0: + if len(errors) > 0: raise ValueErrorsException(errors) - - return True diff --git a/AutoscoperM/CMakeLists.txt b/AutoscoperM/CMakeLists.txt index 29d5949..f84983a 100644 --- a/AutoscoperM/CMakeLists.txt +++ b/AutoscoperM/CMakeLists.txt @@ -5,9 +5,9 @@ set(MODULE_NAME AutoscoperM) set(MODULE_PYTHON_SCRIPTS ${MODULE_NAME}.py ${MODULE_NAME}Lib/__init__.py - ${MODULE_NAME}Lib/ErrorUtils.py ${MODULE_NAME}Lib/IO.py ${MODULE_NAME}Lib/SubVolumeExtraction.py + ${MODULE_NAME}Lib/Validation.py ) set(MODULE_PYTHON_RESOURCES