From 331af3fc86d3276d2564bf0e3a1cde78e1682675 Mon Sep 17 00:00:00 2001 From: Parker Norton Date: Fri, 8 May 2026 12:19:55 -0600 Subject: [PATCH 01/10] refactor: add missing typing and docstrings for __init__ --- pyPRMS/control/ControlVariable.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pyPRMS/control/ControlVariable.py b/pyPRMS/control/ControlVariable.py index 9b2cb50..2ab0b4b 100644 --- a/pyPRMS/control/ControlVariable.py +++ b/pyPRMS/control/ControlVariable.py @@ -2,6 +2,7 @@ import datetime import numpy as np +import numpy.typing as npt import re from typing import Callable, Dict, List, Optional, Sequence, Union @@ -18,13 +19,15 @@ class ControlVariable(object): # Create date: 2019-04-18 def __init__(self, name: str, - value = None, + value: Optional[Union[npt.NDArray, np.int32, np.float32, np.float64, np.str_]] = None, meta: Optional[Dict] = None, strict: Optional[bool] = True): """Initialize a control variable object. :param name: Name of control variable + :param value: Value(s) of control variable :param meta: Metadata of the control variable + :param strict: Enforce use of valid control variable metadata """ self.__name = name From 0633d1c15f68cd348f55d9333720f596734b6b15 Mon Sep 17 00:00:00 2001 From: Parker Norton Date: Fri, 8 May 2026 12:21:39 -0600 Subject: [PATCH 02/10] refactor: removed unused imports and comments --- pyPRMS/control/Control.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/pyPRMS/control/Control.py b/pyPRMS/control/Control.py index 051889e..15098f1 100644 --- a/pyPRMS/control/Control.py +++ b/pyPRMS/control/Control.py @@ -1,19 +1,12 @@ #!/usr/bin/env python3 -import io import numpy as np import operator import pandas as pd # type: ignore -import pkgutil import re -import xml.etree.ElementTree as xmlET from typing import Dict, List, Optional, Sequence, Union # OrderedDict as OrderedDictType, -from networkx.utils.misc import check_create_using - -# from rich import pretty -# from rich.console import Console from rich.table import Table from .ControlVariable import ControlVariable From f64aa152db6833ef131755bda3b4ae3dae32241b Mon Sep 17 00:00:00 2001 From: Parker Norton Date: Fri, 8 May 2026 12:24:40 -0600 Subject: [PATCH 03/10] refactor: remove duplicated cond_check dictionary and use prms_helpers --- pyPRMS/control/Control.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pyPRMS/control/Control.py b/pyPRMS/control/Control.py index 15098f1..d0e4f3f 100644 --- a/pyPRMS/control/Control.py +++ b/pyPRMS/control/Control.py @@ -14,13 +14,10 @@ from ..constants import (ctl_order, ctl_implicit_modules, internal_module_map, MetaDataType, VAR_DELIM, PTYPE_TO_PRMS_TYPE) from ..base.console import get_console_instance +from pyPRMS.prms_helpers import cond_check con = None -cond_check = {'=': operator.eq, - '>': operator.gt, - '<': operator.lt} - class Control(object): """ Class object for a collection of control variables. From e5cb1be3d7c5b50b453cd1f813bed9bc3b830e32 Mon Sep 17 00:00:00 2001 From: Parker Norton Date: Fri, 8 May 2026 12:28:06 -0600 Subject: [PATCH 04/10] refactor: fix typo in cbh_files --- pyPRMS/control/Control.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyPRMS/control/Control.py b/pyPRMS/control/Control.py index d0e4f3f..fe54807 100644 --- a/pyPRMS/control/Control.py +++ b/pyPRMS/control/Control.py @@ -87,7 +87,7 @@ def cbh_files(self) -> List[str]: """ # List of control variables that specify possible CBH files - ctl_cbh_files = ['albebo_day', 'cloud_cover_day', 'humidity_day', 'potet_day', 'precip_day', + ctl_cbh_files = ['albedo_day', 'cloud_cover_day', 'humidity_day', 'potet_day', 'precip_day', 'swrad_day', 'tmax_day', 'tmin_day', 'transp_day', 'windspeed_day'] cbh_files = [] From e440e57ca24f938f0ab20aa88714acd0b112dea0 Mon Sep 17 00:00:00 2001 From: Parker Norton Date: Fri, 8 May 2026 12:37:18 -0600 Subject: [PATCH 05/10] refactor: added albedo.day to test_cbh_files expected --- tests/func/test_Control.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/func/test_Control.py b/tests/func/test_Control.py index f98bcb4..7519530 100644 --- a/tests/func/test_Control.py +++ b/tests/func/test_Control.py @@ -226,7 +226,8 @@ def test_set_header_with_none(self, control_object): def test_cbh_files(self, control_object): """Check the default set of CBH files""" - expected = ['cloudcover.day', + expected = ['albedo.day', + 'cloudcover.day', 'humidity.day', 'potet.day', 'precip.day', From 024dd97e78f12fb30581cef153987e194a582cde Mon Sep 17 00:00:00 2001 From: Parker Norton Date: Fri, 8 May 2026 12:39:36 -0600 Subject: [PATCH 06/10] refactor: use context manager for file writing in Control.write() --- pyPRMS/control/Control.py | 100 ++++++++++++++++++-------------------- 1 file changed, 48 insertions(+), 52 deletions(-) diff --git a/pyPRMS/control/Control.py b/pyPRMS/control/Control.py index fe54807..0db6b65 100644 --- a/pyPRMS/control/Control.py +++ b/pyPRMS/control/Control.py @@ -286,59 +286,55 @@ def write(self, filename: str): :param filename: Name of control file to create """ - outfile = open(filename, 'w') - - if self.__header is not None: - for hh in self.__header: - outfile.write(f'{hh}\n') - - order = ['datatype', 'values'] - - # Get set of variables in ctl_order that are missing from control_vars - setdiff = set(self.__control_vars.keys()).difference(set(ctl_order)) - - # Add missing control variables (setdiff) in ctl_order to the end of the list - ctl_order.extend(list(setdiff)) - - for kk in ctl_order: - if self.exists(kk): - cvar = self.get(kk) - - outfile.write(f'{VAR_DELIM}\n') - outfile.write(f'{kk}\n') - - for item in order: - if cvar.meta['datatype'] == 'datetime': - date_tmp = [int(xx) for xx in re.split(r'[-T:.]+', str(cvar.values))[0:6]] - - if item == 'datatype': - outfile.write(f'{len(date_tmp)}\n') - outfile.write(f'{PTYPE_TO_PRMS_TYPE[cvar.meta["datatype"]]}\n') - if item == 'values': - for cval in date_tmp: - outfile.write(f'{cval}\n') - else: - if item == 'datatype': - outfile.write(f'{cvar.size}\n') - outfile.write(f'{PTYPE_TO_PRMS_TYPE[cvar.meta["datatype"]]}\n') - if item == 'values': - if cvar.meta['context'] == 'scalar': - # Single-values (e.g. int, float, str) - # print(type(cvar.values)) - if isinstance(cvar.values, np.bytes_): - print("BYTES") - outfile.write(f'{cvar.values.decode()}\n') + with open(filename, 'w') as outfile: + if self.__header is not None: + for hh in self.__header: + outfile.write(f'{hh}\n') + + order = ['datatype', 'values'] + + # Get set of variables in ctl_order that are missing from control_vars + setdiff = set(self.__control_vars.keys()).difference(set(ctl_order)) + + # Add missing control variables (setdiff) in ctl_order to the end of the list + ctl_order.extend(list(setdiff)) + + for kk in ctl_order: + if self.exists(kk): + cvar = self.get(kk) + + outfile.write(f'{VAR_DELIM}\n') + outfile.write(f'{kk}\n') + + for item in order: + if cvar.meta['datatype'] == 'datetime': + date_tmp = [int(xx) for xx in re.split(r'[-T:.]+', str(cvar.values))[0:6]] + + if item == 'datatype': + outfile.write(f'{len(date_tmp)}\n') + outfile.write(f'{PTYPE_TO_PRMS_TYPE[cvar.meta["datatype"]]}\n') + if item == 'values': + for cval in date_tmp: + outfile.write(f'{cval}\n') + else: + if item == 'datatype': + outfile.write(f'{cvar.size}\n') + outfile.write(f'{PTYPE_TO_PRMS_TYPE[cvar.meta["datatype"]]}\n') + if item == 'values': + if cvar.meta['context'] == 'scalar': + # Single-values (e.g. int, float, str) + if isinstance(cvar.values, np.bytes_): + print("BYTES") + outfile.write(f'{cvar.values.decode()}\n') + else: + outfile.write(f'{cvar.values}\n') else: - outfile.write(f'{cvar.values}\n') - else: - # Multiple-values - if isinstance(cvar.values, np.ndarray): - for cval in cvar.values: - outfile.write(f'{cval}\n') - else: - outfile.write(f'{cvar.values}\n') - - outfile.close() + # Multiple-values + if isinstance(cvar.values, np.ndarray): + for cval in cvar.values: + outfile.write(f'{cval}\n') + else: + outfile.write(f'{cvar.values}\n') def write_metadata_csv(self, filename: str, sep: str = '\t'): """Writes the control metadata to a CSV file""" From 03508ab4546924fa2a7e6d1733d61f919958cce5 Mon Sep 17 00:00:00 2001 From: Parker Norton Date: Fri, 8 May 2026 12:41:21 -0600 Subject: [PATCH 07/10] refactor: replace assert False with raise NotImplementedError in Control._read() --- pyPRMS/control/Control.py | 2 +- tests/func/test_Control.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyPRMS/control/Control.py b/pyPRMS/control/Control.py index 0db6b65..23a436f 100644 --- a/pyPRMS/control/Control.py +++ b/pyPRMS/control/Control.py @@ -397,7 +397,7 @@ def _check_condition(self, cstr: str) -> bool: def _read(self): """Abstract function for reading. """ - assert False, 'Control._read() must be defined by child class' + raise NotImplementedError('Control._read() must be defined by child class') def _preload_metadata(self): # Create an entry for each variable in the control section of diff --git a/tests/func/test_Control.py b/tests/func/test_Control.py index 7519530..33043d9 100644 --- a/tests/func/test_Control.py +++ b/tests/func/test_Control.py @@ -199,7 +199,7 @@ def test_to_dict(self, control_object): def test_control_read_method_is_abstract(self, control_object): """The Control class _read() method is abstract""" - with pytest.raises(AssertionError): + with pytest.raises(NotImplementedError): control_object._read() def test_default_header(self, control_object): From 7ed7195297033b89dc1292d32ddc0ff0c209cfd2 Mon Sep 17 00:00:00 2001 From: Parker Norton Date: Fri, 8 May 2026 12:49:53 -0600 Subject: [PATCH 08/10] refactor: fix filename property return type to Path in ControlFile --- pyPRMS/control/ControlFile.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pyPRMS/control/ControlFile.py b/pyPRMS/control/ControlFile.py index 670ef5a..b614f70 100644 --- a/pyPRMS/control/ControlFile.py +++ b/pyPRMS/control/ControlFile.py @@ -34,12 +34,10 @@ def __init__(self, filename: Union[str, Path], self.__isloaded = False self.__include_missing = include_missing - if isinstance(filename, str): - filename = Path(filename) self.filename = filename @property - def filename(self) -> Union[str, Path]: + def filename(self) -> Path: """Get control filename. :returns: Name of control file @@ -54,7 +52,7 @@ def filename(self, filename: Union[str, Path]): """ self.__isloaded = False - self.__filename = filename + self.__filename = filename if isinstance(filename, Path) else Path(filename) self._read() def _read(self): From aa2823011dadb9969ef683a4274415b73aceb79d Mon Sep 17 00:00:00 2001 From: Parker Norton Date: Fri, 8 May 2026 12:52:42 -0600 Subject: [PATCH 09/10] refactor: removed stale docstring in add() --- pyPRMS/control/Control.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pyPRMS/control/Control.py b/pyPRMS/control/Control.py index 23a436f..a2eda65 100644 --- a/pyPRMS/control/Control.py +++ b/pyPRMS/control/Control.py @@ -189,7 +189,6 @@ def add(self, name: str): # , meta=None): """Add a control variable by name. :param name: Name of the control variable - :param datatype: The datatype of the control variable :raises ControlError: if control variable already exists """ From 12a6415f121a6c26e14d91b2125057f3becf3bac Mon Sep 17 00:00:00 2001 From: Parker Norton Date: Fri, 8 May 2026 12:57:20 -0600 Subject: [PATCH 10/10] refactor: simplify _value_meaning_test and remove unused re import in ControlVariable --- pyPRMS/control/ControlVariable.py | 52 +++++++++++-------------------- 1 file changed, 18 insertions(+), 34 deletions(-) diff --git a/pyPRMS/control/ControlVariable.py b/pyPRMS/control/ControlVariable.py index 2ab0b4b..a313bd9 100644 --- a/pyPRMS/control/ControlVariable.py +++ b/pyPRMS/control/ControlVariable.py @@ -3,7 +3,6 @@ import datetime import numpy as np import numpy.typing as npt -import re from typing import Callable, Dict, List, Optional, Sequence, Union from ..constants import NEW_PTYPE_TO_DTYPE @@ -175,37 +174,22 @@ def value_meaning(self) -> Union[str, None]: # return meaning.get(self.values, meaning.get(str(self.values), None)) def _value_meaning_test(self, key, src_dict): - try: + """Look up the meaning for a value in a dictionary that may use + direct keys or conditional keys (e.g. '>0', '<5'). + """ + # Direct lookup by value + if key in src_dict: return src_dict[key] - except KeyError: - # Maybe the key is a string - try: - return src_dict[str(key)] - except KeyError: - # Maybe one of the keys is a conditional? - patterns = ['[><]'] - regex = [re.compile('^' + pat).match for pat in patterns] - - tt = {kk: vv for kk, vv in src_dict.items() - if any (reg(kk) for reg in regex)} - - if len(tt) > 0: - # So there is a conditional - for mm in tt: - # print(mm.split()) - if cond_check[mm[0]](key, int(mm[1:])): - return src_dict[mm] - # print(f'{mm}: {src_dict[mm]}') - raise ValueError('Invalid control value') - - - # try: - # if 'valid_values' in self.meta: - # # We want a KeyError here if the key is missing - # return self.meta['valid_values'][self.values] - # - # return None - # except KeyError: - # # Try again but return None if the key is still missing - # return self.meta['valid_values'].get(str(self.values), None) - # return None \ No newline at end of file + + # Try string representation of the key + str_key = str(key) + if str_key in src_dict: + return src_dict[str_key] + + # Check conditional keys (e.g. ">0", "<100") + for cond_key, meaning in src_dict.items(): + if cond_key and cond_key[0] in '><': + if cond_check[cond_key[0]](key, int(cond_key[1:])): + return meaning + + raise ValueError('Invalid control value') \ No newline at end of file