From 2bf56e3375847055f58c539d207a2fb7c69e8505 Mon Sep 17 00:00:00 2001 From: Dusko Mirkovic Date: Fri, 14 Jul 2023 14:27:09 +0200 Subject: [PATCH] CTX-3757: Added validation and parsing of enum and list[enum] parameter types --- coretex/coretex/experiment/__init__.py | 1 + coretex/coretex/experiment/parameters.py | 165 ++++++++++++++++------- coretex/project/local.py | 27 +++- 3 files changed, 140 insertions(+), 53 deletions(-) diff --git a/coretex/coretex/experiment/__init__.py b/coretex/coretex/experiment/__init__.py index 76d45455..caff0338 100644 --- a/coretex/coretex/experiment/__init__.py +++ b/coretex/coretex/experiment/__init__.py @@ -20,3 +20,4 @@ from .status import ExperimentStatus from .experiment_builder import ExperimentBuilder from .metrics import Metric, MetricType +from .parameters import ExperimentParameter, ExperimentParameterType diff --git a/coretex/coretex/experiment/parameters.py b/coretex/coretex/experiment/parameters.py index 8a2e54ea..8a8fe927 100644 --- a/coretex/coretex/experiment/parameters.py +++ b/coretex/coretex/experiment/parameters.py @@ -15,13 +15,9 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . -from __future__ import annotations - from typing import Any, Type, List, Dict from enum import IntEnum -import json - from .utils import getDatasetType, fetchDataset from ..space import SpaceTask from ...codable import Codable @@ -38,9 +34,11 @@ class ExperimentParameterType(IntEnum): floatList = 7 strList = 8 imuVectors = 9 + enum = 10 + enumList = 11 @staticmethod - def fromStringValue(stringValue: str) -> ExperimentParameterType: + def fromStringValue(stringValue: str) -> 'ExperimentParameterType': for value in ExperimentParameterType: if value.stringValue == stringValue: return value @@ -76,6 +74,12 @@ def stringValue(self) -> str: if self == ExperimentParameterType.imuVectors: return "IMUVectors" + if self == ExperimentParameterType.enum: + return "enum" + + if self == ExperimentParameterType.enumList: + return "list[enum]" + raise ValueError(f">> [Coretex] Unsupported type: {self}") @property @@ -101,6 +105,12 @@ def types(self) -> List[Type]: # parameters of type IMUVectors have dictionary as value return [dict] + if self == ExperimentParameterType.enum: + return [dict] + + if self == ExperimentParameterType.enumList: + return [dict] + raise ValueError(f">> [Coretex] Unsupported type: {self}") @property @@ -108,7 +118,8 @@ def isList(self) -> bool: return self in [ ExperimentParameterType.intList, ExperimentParameterType.floatList, - ExperimentParameterType.strList + ExperimentParameterType.strList, + ExperimentParameterType.enumList ] @property @@ -128,6 +139,97 @@ def listType(self) -> Type: raise ValueError(f">> [Coretex] Unsupported type: {self}") +class ParameterError(Exception): + + def __init__(self, message: str) -> None: + super().__init__(f">> [Coretex] {message}") + + @staticmethod + def type(parameter: 'ExperimentParameter') -> 'ParameterError': + expected = parameter.dataType.stringValue + received = parameter.generateTypeDescription() + + return ParameterError(f"Parameter \"{parameter.name}\" has invalid type. Expected \"{expected}\", got \"{received}\"") + + +def _validateGeneric(parameter: 'ExperimentParameter') -> None: + if parameter.required and parameter.value is None: + raise ParameterError.type(parameter) + + if not parameter.required and parameter.value is None: + return + + if parameter.dataType.isList: + if not isinstance(parameter.value, list): + raise ParameterError.type(parameter) + + if not all(isinstance(element, parameter.dataType.listType) for element in parameter.value): + raise ParameterError.type(parameter) + else: + # Dataset parameter is an integer under the hood, and in python bool is a subclass + # of integer. To avoid assinging boolean values to dataset parameters we have to explicitly + # check if the value which was passed in for dataset is a bool. + if parameter.dataType == ExperimentParameterType.dataset and isinstance(parameter.value, bool): + raise ParameterError.type(parameter) + + if not any(isinstance(parameter.value, dataType) for dataType in parameter.dataType.types): + raise ParameterError.type(parameter) + + +def _validateEnumValue(parameter: 'ExperimentParameter') -> None: + value = parameter.value + + # Enum parameter must be a dict + if not isinstance(value, dict): + raise ParameterError.type(parameter) + + # Enum parameter must contain 2 key-value pairs: selected and options + if len(value) != 2 or "options" not in value or "selected" not in value: + keys = ", ".join(value.keys()) + raise ParameterError(f"Enum parameter \"{parameter.name}\" must contain only \"selected\" and \"options\" properties, but it contains \"{keys}\"") + + options = value.get("options") + + # options must be an object of type list + if not isinstance(options, list): + raise ParameterError(f"Enum parameter \"{parameter.name}.options\" has invalid type. Expected \"list[str]\", got \"{type(options).__name__}\"") + + # all elements of options list must be strings + if not all(isinstance(element, str) for element in options): + elementTypes = ", ".join({type(element).__name__ for element in options}) + raise ParameterError(f"Elements of enum parameter \"{parameter.name}.options\" have invalid type. Expected \"list[str]\" got \"list[{elementTypes}]\"") + + # options elements must not be empty strings + if not all(element != "" for element in options): + raise ParameterError(f"Elements of enum parameter \"{parameter.name}.options\" must be non-empty strings.") + + selected = value.get("selected") + + if selected is None and parameter.required: + raise ParameterError(f"Enum parameter \"{parameter.name}.selected\" has invalid type. Expected \"int\", got \"{type(selected).__name__}\"") + + if selected is None and not parameter.required: + return + + if parameter.dataType.isList: + if not isinstance(selected, list): + raise ParameterError(f"Enum list parameter \"{parameter.name}.selected\" has invalid type. Expected \"list[int]\", got \"{type(selected).__name__}\"") + + if not all(isinstance(element, int) for element in selected): + elementTypes = ", ".join({type(element).__name__ for element in selected}) + raise ParameterError(f"Enum list parameter \"{parameter.name}.selected\" has invalid type. Expected \"list[int]\", got \"list[{elementTypes}]\"") + + invalidIndxCount = len([element for element in selected if element >= len(options) or element < 0]) + if invalidIndxCount > 0: + raise ParameterError(f"Enum list parameter \"{parameter.name}.selected\" has out of range values") + else: + if not isinstance(selected, int): + raise ParameterError(f"Enum parameter \"{parameter.name}.selected\" has invalid type. Expected \"int\", got \"{type(selected).__name__}\"") + + if selected >= len(options) or selected < 0: + raise ParameterError(f"Enum parameter \"{parameter.name}.selected\" has out of range value") + + class ExperimentParameter(Codable): name: str @@ -152,26 +254,16 @@ def _decodeValue(cls, key: str, value: Any) -> Any: return super()._decodeValue(key, value) - def isValid(self) -> bool: - if not self.required and self.value is None: - return True - - if self.dataType.isList: - if not isinstance(self.value, list): - return False + def onDecode(self) -> None: + super().onDecode() - return all( - isinstance(element, self.dataType.listType) - for element in self.value - ) + self.__validate() - # Dataset parameter is an integer under the hood, and in python bool is a subclass - # of integer. To avoid assinging boolean values to dataset parameters we have to explicitly - # check if the value which was passed in for dataset is a bool. - if self.dataType == ExperimentParameterType.dataset and isinstance(self.value, bool): - return False - - return any(isinstance(self.value, dataType) for dataType in self.dataType.types) + def __validate(self) -> None: + if self.dataType == ExperimentParameterType.enum or self.dataType == ExperimentParameterType.enumList: + _validateEnumValue(self) + else: + _validateGeneric(self) def generateTypeDescription(self) -> str: if not self.dataType.isList or self.value is None: @@ -180,29 +272,6 @@ def generateTypeDescription(self) -> str: elementTypes = ", ".join({type(value).__name__ for value in self.value}) return f"list[{elementTypes}]" - @staticmethod - def readExperimentConfig() -> List[ExperimentParameter]: - parameters: List[ExperimentParameter] = [] - - with open("./experiment.config", "rb") as configFile: - configContent: Dict[str, Any] = json.load(configFile) - parametersJson = configContent["parameters"] - - if not isinstance(parametersJson, list): - raise ValueError(">> [Coretex] Invalid experiment.config file. Property 'parameters' must be an array") - - for parameterJson in parametersJson: - parameter = ExperimentParameter.decode(parameterJson) - if not parameter.isValid(): - expected = parameter.dataType.stringValue - received = parameter.generateTypeDescription() - - raise ValueError(f">> [Coretex] Parameter \"{parameter.name}\" has invalid type. Expected \"{expected}\", got \"{received}\"") - - parameters.append(parameter) - - return parameters - def parseParameters(parameters: List[ExperimentParameter], task: SpaceTask) -> Dict[str, Any]: values: Dict[str, Any] = {} @@ -222,6 +291,8 @@ def parseParameters(parameters: List[ExperimentParameter], task: SpaceTask) -> D raise ValueError(f">> [Coretex] Failed to fetch dataset with ID: {parameter.value}") values[parameter.name] = dataset + elif parameter.dataType == ExperimentParameterType.enum: + values[parameter.name] = parameter.value["options"][parameter.value["selected"]] else: values[parameter.name] = parameter.value diff --git a/coretex/project/local.py b/coretex/project/local.py index 6a80144b..0e34d9b4 100644 --- a/coretex/project/local.py +++ b/coretex/project/local.py @@ -15,21 +15,21 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . -from typing import Tuple, Optional, List +from typing import Tuple, Optional, List, Dict, Any from getpass import getpass import logging import os +import json from tap import Tap import psutil from .base import ProjectCallback -from ..coretex import Experiment, ExperimentStatus +from ..coretex import Experiment, ExperimentStatus, ExperimentParameter from ..folder_management import FolderManager from ..networking import networkManager -from ..coretex.experiment.parameters import ExperimentParameter class LocalProjectCallback(ProjectCallback): @@ -88,6 +88,23 @@ def configure(self) -> None: self.add_argument("--description", nargs = "?", type = str, default = None) +def _readExperimentConfig() -> List['ExperimentParameter']: + parameters: List[ExperimentParameter] = [] + + with open("./experiment.config", "rb") as configFile: + configContent: Dict[str, Any] = json.load(configFile) + parametersJson = configContent["parameters"] + + if not isinstance(parametersJson, list): + raise ValueError(">> [Coretex] Invalid experiment.config file. Property 'parameters' must be an array") + + for parameterJson in parametersJson: + parameter = ExperimentParameter.decode(parameterJson) + parameters.append(parameter) + + return parameters + + def processLocal(args: Optional[List[str]] = None) -> Tuple[int, ProjectCallback]: parser, unknown = LocalArgumentParser().parse_known_args(args) @@ -111,8 +128,6 @@ def processLocal(args: Optional[List[str]] = None) -> Tuple[int, ProjectCallback if not os.path.exists("experiment.config"): raise FileNotFoundError(">> [Coretex] \"experiment.config\" file not found") - parameters = ExperimentParameter.readExperimentConfig() - experiment: Experiment = Experiment.run( parser.projectId, # Dummy Local node ID, hardcoded as it is only a temporary solution, @@ -121,7 +136,7 @@ def processLocal(args: Optional[List[str]] = None) -> Tuple[int, ProjectCallback -1, parser.name, parser.description, - parameters = [parameter.encode() for parameter in parameters] + parameters = [parameter.encode() for parameter in _readExperimentConfig()] ) logging.getLogger("coretexpylib").info(f">> [Coretex] Created local experiment with ID \"{experiment.id}\"")