chore: copilot review changes

Author: lalepee

cosmotech/coal/cosmotech_api/apis/dataset.py

@@ -79,7 +79,7 @@ def _download_part(self, dataset_id, dataset_part, destination):
         )
 
     @staticmethod
-    def path_to_parts(_path, part_type) -> list[tuple[str, str, Path, DatasetPartTypeEnum]]:
+    def path_to_parts(_path, part_type) -> list[tuple[str, Path, DatasetPartTypeEnum]]:
         if (_path := Path(_path)).is_dir():
             return list((str(_p.relative_to(_path)), _p, part_type) for _p in _path.rglob("*") if _p.is_file())
         return list(((_path.name, _path, part_type),))

cosmotech/coal/store/csv.py

@@ -16,8 +16,10 @@ def store_csv_file(
     table_name: str,
     csv_path: pathlib.Path,
     replace_existsing_file: bool = False,
-    store=Store(),
+    store: Store | None = None,
 ):
+    if store is None:
+        store = Store()
     if not csv_path.exists():
         raise FileNotFoundError(f"File {csv_path} does not exists")
 
@@ -32,8 +34,10 @@ def convert_store_table_to_csv(
     table_name: str,
     csv_path: pathlib.Path,
     replace_existsing_file: bool = False,
-    store=Store(),
+    store: Store | None = None,
 ):
+    if store is None:
+        store = Store()
     if csv_path.name.endswith(".csv") and csv_path.exists() and not replace_existsing_file:
         raise FileExistsError(f"File {csv_path} already exists")
     if not csv_path.name.endswith(".csv"):

cosmotech/coal/store/native_python.py

@@ -14,12 +14,16 @@ def store_pylist(
     table_name: str,
     data: list[dict],
     replace_existsing_file: bool = False,
-    store=Store(),
+    store: Store | None = None,
 ):
+    if store is None:
+        store = Store()
     data = pa.Table.from_pylist(data)
 
     store.add_table(table_name=table_name, data=data, replace=replace_existsing_file)
 
 
-def convert_table_as_pylist(table_name: str, store=Store()):
+def convert_table_as_pylist(table_name: str, store: Store | None = None):
+    if store is None:
+        store = Store()
     return store.get_table(table_name).to_pylist()

cosmotech/coal/store/pandas.py

@@ -15,12 +15,16 @@ def store_dataframe(
     table_name: str,
     dataframe: pd.DataFrame,
     replace_existsing_file: bool = False,
-    store=Store(),
+    store: Store | None = None,
 ):
+    if store is None:
+        store = Store()
     data = pyarrow.Table.from_pandas(dataframe)
 
     store.add_table(table_name=table_name, data=data, replace=replace_existsing_file)
 
 
-def convert_store_table_to_dataframe(table_name: str, store=Store()) -> pd.DataFrame:
+def convert_store_table_to_dataframe(table_name: str, store: Store | None = None) -> pd.DataFrame:
+    if store is None:
+        store = Store()
     return store.get_table(table_name).to_pandas()

cosmotech/coal/store/parquet.py

@@ -17,8 +17,10 @@ def store_parquet_file(
     table_name: str,
     parquet_path: pathlib.Path,
     replace_existsing_file: bool = False,
-    store=Store(),
+    store: Store | None = None,
 ):
+    if store is None:
+        store = Store()
     if not parquet_path.exists():
         raise FileNotFoundError(f"File {parquet_path} does not exists")
 
@@ -33,8 +35,10 @@ def convert_store_table_to_parquet(
     table_name: str,
     parquet_path: pathlib.Path,
     replace_existsing_file: bool = False,
-    store=Store(),
+    store: Store | None = None,
 ):
+    if store is None:
+        store = Store()
     if parquet_path.name.endswith(".parquet") and parquet_path.exists() and not replace_existsing_file:
         raise FileExistsError(f"File {parquet_path} already exists")
     if not parquet_path.name.endswith(".parquet"):

cosmotech/coal/store/pyarrow.py

@@ -14,10 +14,14 @@ def store_table(
     table_name: str,
     data: pa.Table,
     replace_existsing_file: bool = False,
-    store=Store(),
+    store: Store | None = None,
 ):
+    if store is None:
+        store = Store()
     store.add_table(table_name=table_name, data=data, replace=replace_existsing_file)
 
 
-def convert_store_table_to_dataframe(table_name: str, store=Store()) -> pa.Table:
+def convert_store_table_to_dataframe(table_name: str, store: Store | None = None) -> pa.Table:
+    if store is None:
+        store = Store()
     return store.get_table(table_name)

cosmotech/coal/utils/input_collector.py

@@ -21,7 +21,7 @@ def fetch_parameter(self, param_name: str) -> Path:
     def fetch_workspace_file(self, file_name: str) -> Path:
         return self.workspace_collector.fetch(file_name)
 
-    def fetch(self, name: str) -> Path:
+    def fetch(self, name: str) -> Path | str:
         try:
             return self.fetch_parameter(name)
         except (KeyError, FileNotFoundError):
@@ -38,8 +38,11 @@ def __init__(self):
         self.paths: dict[str, Path] = {}
 
     def collect(self):
-        for dataset_id in os.listdir(EC.cosmotech.dataset_absolute_path):
-            for r, d, f in os.walk(Path(EC.cosmotech.dataset_absolute_path) / dataset_id):
+        base_path = Path(EC.cosmotech.dataset_absolute_path)
+        for dataset_id in os.listdir(base_path):
+            if not (base_path / dataset_id).is_dir():
+                continue
+            for r, d, f in os.walk(base_path / dataset_id):
                 for dataset_name in f:
                     path = Path(r) / dataset_name
                     self.paths[dataset_name] = path
@@ -89,15 +92,18 @@ def read_parameters_json(self):
                     self.parameters[parameter["parameterId"]] = parameter["value"]
 
     def collect(self):
-        for dataset_id in os.listdir(EC.cosmotech.parameters_absolute_path):
-            for r, d, f in os.walk(Path(EC.cosmotech.parameters_absolute_path) / dataset_id):
+        base_path = Path(EC.cosmotech.parameters_absolute_path)
+        for dataset_id in os.listdir(base_path):
+            if not (base_path / dataset_id).is_dir():
+                continue
+            for r, d, f in os.walk(base_path / dataset_id):
                 for file_name in f:
                     path = Path(r) / file_name
                     param_name = path.parent.name
                     self.paths[param_name] = path
                     self.paths[path.stem] = path
 
-    def fetch_parameter(self, param_name: str) -> Path:
+    def fetch_parameter(self, param_name: str) -> Path | str:
         # lazy collection to avoid unnecessary json loading
         if not self.parameters:
             self.read_parameters_json()
@@ -111,7 +117,7 @@ def fetch_file_path(self, param_name: str) -> Path:
             return self.paths[param_name]
         raise FileNotFoundError(f"File for {param_name} not found in {EC.cosmotech.parameters_absolute_path}.")
 
-    def fetch(self, param_name: str) -> Path:
+    def fetch(self, param_name: str) -> Path | str:
         try:
             return self.fetch_parameter(param_name)
         except KeyError:

cosmotech/csm_data/commands/store/load_csv_folder.py

@@ -41,9 +41,9 @@ def load_csv_folder(store_folder, csv_folder):
     from cosmotech.coal.utils.logger import LOGGER
 
     _conf = Configuration()
-
     _conf.coal.store = store_folder
 
+    store = Store(False, _conf)
     for csv_path in pathlib.Path(csv_folder).glob("*.csv"):
         LOGGER.info(T("coal.services.azure_storage.found_file").format(file=csv_path.name))
-        store_csv_file(csv_path.name[:-4], csv_path, store=Store(False, _conf))
+        store_csv_file(csv_path.name[:-4], csv_path, store=store)

cosmotech/csm_data/commands/store/load_parquet_folder.py

@@ -41,9 +41,9 @@ def load_parquet_folder(store_folder, parquet_folder):
     from cosmotech.coal.utils.logger import LOGGER
 
     _conf = Configuration()
-
     _conf.coal.store = store_folder
 
+    store = Store(False, _conf)
     for parquet_path in pathlib.Path(parquet_folder).glob("*.parquet"):
         LOGGER.info(T("coal.services.azure_storage.found_file").format(file=parquet_path.name))
-        store_parquet_file(parquet_path.name[:-8], parquet_path, store=Store(False, _conf))
+        store_parquet_file(parquet_path.name[:-8], parquet_path, store=store)

tests/unit/coal/test_utils/test_utils_input_collector.py

@@ -164,7 +164,7 @@ def test_fetch_returns_value_if_already_in_parameters(self, tmp_path, mock_ec):
         assert result == "value"
 
     def test_fetch_falls_back_to_file_without_lazy_load(self, tmp_path, mock_ec):
-        """fetch() does NOT trigger read_parameters_json — falls straight to fetch_file_path."""
+        """fetch() delegates to fetch_parameter(), which loads parameters.json FIRST and returns the JSON value."""
         param_dir = tmp_path / "myparam"
         param_dir.mkdir()
         f = param_dir / "data.csv"