diff --git a/model_manager/sientia/models.py b/model_manager/sientia/models.py index 5277bae..677caec 100644 --- a/model_manager/sientia/models.py +++ b/model_manager/sientia/models.py @@ -17,14 +17,35 @@ NORMALIZATION = 'Normalization' FEATURE_CREATION = 'Feature Creation' LAG_CREATION = 'Lag Creation' +# Allowed frontend date formats and their strftime equivalents (single source of truth) +FRONTEND_DATE_FORMAT_TO_STRFTIME = { + 'dd/MM/yyyy HH:mm:ss': '%d/%m/%Y %H:%M:%S', + 'MM/dd/yyyy HH:mm:ss': '%m/%d/%Y %H:%M:%S', + 'yyyy/MM/dd HH:mm:ss': '%Y/%m/%d %H:%M:%S', + 'dd-MM-yyyy HH:mm:ss': '%d-%m-%Y %H:%M:%S', + 'MM-dd-yyyy HH:mm:ss': '%m-%d-%Y %H:%M:%S', + 'yyyy-MM-dd HH:mm:ss': '%Y-%m-%d %H:%M:%S', +} +ALLOWED_FRONTEND_DATE_FORMATS = frozenset(FRONTEND_DATE_FORMAT_TO_STRFTIME.keys()) + + +def validate_frontend_date_format(fmt: str | None) -> None: + """Raise ValueError if fmt is set and not one of the allowed frontend date formats.""" + if not fmt or not fmt.strip(): + return + if fmt not in ALLOWED_FRONTEND_DATE_FORMATS: + allowed = ', '.join(sorted(ALLOWED_FRONTEND_DATE_FORMATS)) + raise ValueError( + f'Invalid date_format "{fmt}". Allowed formats: {allowed}' + ) + def _frontend_date_format_to_strftime(fmt: str | None) -> str | None: - """Convert front-end date format (e.g. dd/MM/yyyy HH:mm:ss) to Python strftime.""" + """Convert front-end date format to Python strftime. Validates format; returns None for empty.""" if not fmt: return None - out = fmt.replace('yyyy', '%Y').replace('MM', '%m').replace('dd', '%d') - out = out.replace('HH', '%H').replace('mm', '%M').replace('ss', '%S') - return out + validate_frontend_date_format(fmt) + return FRONTEND_DATE_FORMAT_TO_STRFTIME[fmt] class LinearRegressionModel(BaseEstimator, TransformerMixin): diff --git a/model_manager/utils/models/train_model_params.py b/model_manager/utils/models/train_model_params.py index f79b536..87a182d 100644 --- a/model_manager/utils/models/train_model_params.py +++ b/model_manager/utils/models/train_model_params.py @@ -1,6 +1,8 @@ from dataclasses import dataclass from typing import Any +from model_manager.sientia.models import validate_frontend_date_format + # Model name constants MODEL_LINEAR_REGRESSION = 'Linear Regression' MODEL_POLYNOMIAL_REGRESSION = 'Polynomial Regression' @@ -217,6 +219,7 @@ class TrainModelParams: self._validate_intervals_and_dates() self._validate_limits() self._validate_required_strings() + self._validate_date_format() def _validate_numeric_ranges(self) -> None: """Validate numeric parameters are within acceptable ranges.""" @@ -327,3 +330,8 @@ class TrainModelParams: if not self.experiment_name.strip(): raise ValueError('experiment_name cannot be empty or whitespace') + + def _validate_date_format(self) -> None: + """Validate date_format is one of the allowed frontend formats when set.""" + if self.date_format: + validate_frontend_date_format(self.date_format) diff --git a/tests/sientia/test_models.py b/tests/sientia/test_models.py index af19119..6ee7805 100644 --- a/tests/sientia/test_models.py +++ b/tests/sientia/test_models.py @@ -7,34 +7,56 @@ import pandas as pd from pytest import raises from model_manager.sientia.models import ( + ALLOWED_FRONTEND_DATE_FORMATS, + FRONTEND_DATE_FORMAT_TO_STRFTIME, DataPreprocessor, LinearRegressionModel, _frontend_date_format_to_strftime, + validate_frontend_date_format, ) class TestFrontendDateFormatToStrftime: """Tests for _frontend_date_format_to_strftime (models module).""" - def test_none_or_empty_returns_none_or_empty(self): - """None or empty string returns None or falsy.""" + def test_none_or_empty_returns_none(self): + """None or empty string returns None.""" assert _frontend_date_format_to_strftime(None) is None assert _frontend_date_format_to_strftime('') is None - def test_dd_mm_yyyy_hh_mm_ss(self): - """Converts dd/MM/yyyy HH:mm:ss to strftime.""" - result = _frontend_date_format_to_strftime('dd/MM/yyyy HH:mm:ss') - assert result == '%d/%m/%Y %H:%M:%S' + def test_all_six_allowed_formats_convert_correctly(self): + """All allowed frontend formats map to expected strftime.""" + for frontend_fmt, strftime_fmt in FRONTEND_DATE_FORMAT_TO_STRFTIME.items(): + assert _frontend_date_format_to_strftime(frontend_fmt) == strftime_fmt - def test_yyyy_mm_dd(self): - """Converts yyyy-MM-dd to strftime.""" - result = _frontend_date_format_to_strftime('yyyy-MM-dd') - assert result == '%Y-%m-%d' + def test_invalid_format_raises(self): + """Invalid format raises ValueError with allowed list in message.""" + with raises(ValueError, match='Invalid date_format'): + _frontend_date_format_to_strftime('yyyy-MM-dd') + with raises(ValueError, match='Allowed formats'): + _frontend_date_format_to_strftime('invalid') - def test_iso_datetime(self): - """Converts yyyy-MM-dd HH:mm:ss to strftime.""" - result = _frontend_date_format_to_strftime('yyyy-MM-dd HH:mm:ss') - assert result == '%Y-%m-%d %H:%M:%S' + +class TestValidateFrontendDateFormat: + """Tests for validate_frontend_date_format.""" + + def test_none_or_empty_does_not_raise(self): + """None or empty string does not raise.""" + validate_frontend_date_format(None) + validate_frontend_date_format('') + validate_frontend_date_format(' ') + + def test_allowed_formats_do_not_raise(self): + """All allowed formats pass validation.""" + for fmt in ALLOWED_FRONTEND_DATE_FORMATS: + validate_frontend_date_format(fmt) + + def test_invalid_format_raises(self): + """Invalid format raises ValueError.""" + with raises(ValueError, match='Invalid date_format'): + validate_frontend_date_format('yyyy-MM-dd') + with raises(ValueError, match='Invalid date_format'): + validate_frontend_date_format('custom-bad-format') class _IterableWithContains: diff --git a/tests/utils/models/test_train_model_params.py b/tests/utils/models/test_train_model_params.py index 5c5443a..e6ce207 100644 --- a/tests/utils/models/test_train_model_params.py +++ b/tests/utils/models/test_train_model_params.py @@ -555,6 +555,27 @@ def test_validate_business_rules_valid_start_and_end_date(valid_train_params_dic params.validate_business_rules() # Should not raise +def test_validate_business_rules_invalid_date_format(valid_train_params_dict): + """Test validate_business_rules raises when date_format is not allowed.""" + from model_manager.utils.models.train_model_params import TrainModelParams + + params = TrainModelParams.from_dict(valid_train_params_dict) + params.date_format = 'yyyy-MM-dd' + + with pytest.raises(ValueError, match='Invalid date_format'): + params.validate_business_rules() + + +def test_validate_business_rules_valid_date_format(valid_train_params_dict): + """Test validate_business_rules accepts allowed date_format.""" + from model_manager.utils.models.train_model_params import TrainModelParams + + params = TrainModelParams.from_dict(valid_train_params_dict) + params.date_format = 'yyyy-MM-dd HH:mm:ss' + + params.validate_business_rules() # Should not raise + + def test_validate_business_rules_polynomial_regression_valid(valid_train_params_dict): """Test validate_business_rules accepts valid Polynomial Regression config.""" from model_manager.utils.models.train_model_params import TrainModelParams diff --git a/tests/utils/repository/test_training_repository.py b/tests/utils/repository/test_training_repository.py index c3ed4b5..e33384b 100644 --- a/tests/utils/repository/test_training_repository.py +++ b/tests/utils/repository/test_training_repository.py @@ -1125,27 +1125,23 @@ class TestExtractModelEquationPolynomial: class TestFrontendFormatToStrftime: - """Tests for _frontend_format_to_strftime.""" + """Tests for _frontend_format_to_strftime (alias from models).""" def test_empty_or_none_returns_none(self): """Empty string or None returns None (same behavior as models module).""" assert _frontend_format_to_strftime('') is None assert _frontend_format_to_strftime(None) is None - def test_dd_mm_yyyy_hh_mm_ss(self): - """Converts dd/MM/yyyy HH:mm:ss to strftime.""" - result = _frontend_format_to_strftime('dd/MM/yyyy HH:mm:ss') - assert result == '%d/%m/%Y %H:%M:%S' + def test_allowed_formats_convert_correctly(self): + """Allowed frontend formats convert to strftime via mapper.""" + assert _frontend_format_to_strftime('dd/MM/yyyy HH:mm:ss') == '%d/%m/%Y %H:%M:%S' + assert _frontend_format_to_strftime('yyyy-MM-dd HH:mm:ss') == '%Y-%m-%d %H:%M:%S' + assert _frontend_format_to_strftime('MM/dd/yyyy HH:mm:ss') == '%m/%d/%Y %H:%M:%S' - def test_yyyy_mm_dd(self): - """Converts yyyy-MM-dd to strftime.""" - result = _frontend_format_to_strftime('yyyy-MM-dd') - assert result == '%Y-%m-%d' - - def test_iso_like_datetime(self): - """Converts yyyy-MM-dd HH:mm:ss to strftime.""" - result = _frontend_format_to_strftime('yyyy-MM-dd HH:mm:ss') - assert result == '%Y-%m-%d %H:%M:%S' + def test_invalid_format_raises(self): + """Invalid date format raises ValueError.""" + with pytest.raises(ValueError, match='Invalid date_format'): + _frontend_format_to_strftime('yyyy-MM-dd') class TestEnsureDateColumnParsed: @@ -1215,8 +1211,11 @@ class TestEnsureDateColumnParsed: def test_invalid_values_coerced_to_nat(self, date_params): """Invalid date strings are coerced to NaT when format is set.""" - date_params.date_format = 'yyyy-MM-dd' - data = pd.DataFrame({'ts': ['2023-01-01', 'not-a-date', '2023-12-31'], 'x': [1, 2, 3]}) + date_params.date_format = 'yyyy-MM-dd HH:mm:ss' + data = pd.DataFrame({ + 'ts': ['2023-01-01 00:00:00', 'not-a-date', '2023-12-31 00:00:00'], + 'x': [1, 2, 3], + }) result = _ensure_date_column_parsed(data, date_params) assert pd.isna(result['ts'].iloc[1]) assert result['ts'].iloc[0].year == 2023