Breaking: fully separate file_keys and file_list configuration path to guarantee proper parsing

Author: francois-drielsma

src/spine/bin/cli.py

@@ -5,22 +5,23 @@
 import os
 import pathlib
 import sys
+from typing import List, Optional
 
 from spine.config import load_config
 from spine.config.loader import parse_value, set_nested_value
 
 
 def main(
-    config,
-    source,
-    source_list,
-    output,
-    n,
-    nskip,
-    log_dir,
-    weight_prefix,
-    weight_path,
-    config_overrides,
+    config: str,
+    source: List[str],
+    source_list: str,
+    output: str,
+    n: int,
+    nskip: int,
+    log_dir: str,
+    weight_prefix: str,
+    weight_path: str,
+    config_overrides: List[str],
 ):
     """Main driver for training/validation/inference/analysis.
 
@@ -47,7 +48,7 @@ def main(
     weight_prefix : str
         Path to the directory for storing the training weights
     weight_path : str
-        Path string a weight file or pattern for multiple weight files to load
+        Path to a weight file or pattern for multiple weight files to load
         the model weights
     config_overrides : List[str]
         List of config overrides in the form "key.path=value"
@@ -76,11 +77,19 @@ def main(
     assert "io" in cfg, "Must provide an `io` block in the configuration."
 
     # Override the input/output command-line information into the configuration
-    if (source is not None and len(source) > 0) or source_list is not None:
+    if source is not None and len(source) > 0:
         if "reader" in cfg["io"]:
-            cfg["io"]["reader"]["file_keys"] = source or source_list
+            cfg["io"]["reader"]["file_keys"] = source
         elif "loader" in cfg["io"]:
-            cfg["io"]["loader"]["dataset"]["file_keys"] = source or source_list
+            cfg["io"]["loader"]["dataset"]["file_keys"] = source
+        else:
+            raise KeyError("Must specify `loader` or `reader` in the `io` block.")
+
+    if source_list is not None:
+        if "reader" in cfg["io"]:
+            cfg["io"]["reader"]["file_list"] = source_list
+        elif "loader" in cfg["io"]:
+            cfg["io"]["loader"]["dataset"]["file_list"] = source_list
         else:
             raise KeyError("Must specify `loader` or `reader` in the `io` block.")
 
@@ -242,15 +251,15 @@ def cli():
     # For actual training/inference, call the main function
     main(
         config=config_file,
-        source=args.source or [],
+        source=args.source,
         source_list=args.source_list,
         output=args.output,
         n=args.iterations,
         nskip=args.nskip,
         log_dir=args.log_dir,
         weight_prefix=args.weight_prefix,
         weight_path=args.weight_path,
-        config_overrides=args.config_overrides or [],
+        config_overrides=args.config_overrides,
     )
 
 

src/spine/io/core/read/base.py

@@ -106,39 +106,44 @@ def get_run_event(self, run: int, subrun: int, event: int) -> Dict[str, Any]:
 
     def process_file_paths(
         self,
-        file_keys: List[str],
+        file_keys: Optional[Union[str, List[str]]] = None,
+        file_list: Optional[str] = None,
         limit_num_files: Optional[int] = None,
         max_print_files: int = 10,
     ) -> None:
         """Process list of files.
 
         Parameters
         ----------
-        file_keys : list
-            List of paths to the HDF5 files to be read
+        file_keys : Union[str, List[str]], optional
+            Path or list of paths to the HDF5 files to be read
+        file_list : str, optional
+            Path to a text file containing a list of file paths to be read
         limit_num_files : int, optional
             Integer limiting number of files to be taken per data directory
         max_print_files : int, default 10
             Maximum number of loaded file names to be printed
         """
         # Some basic checks
-        assert file_keys is not None, "No input `file_keys` provided, abort."
+        assert (file_keys is not None) ^ (
+            file_list is not None
+        ), "Must provide either `file_keys` or `file_list` to process files."
         assert (
             limit_num_files is None or limit_num_files > 0
         ), "If `limit_num_files` is provided, it must be larger than 0."
 
-        # If the file_keys points to a single text file, it must be a text
-        # file containing a list of file paths. Parse it to a list.
-        if isinstance(file_keys, str) and os.path.splitext(file_keys)[-1] == ".txt":
+        # When using a file list in text format, read it and parse to a list
+        if file_list is not None:
             # If the file list is a text file, extract the list of paths
-            assert os.path.isfile(file_keys), (
-                "If the `file_keys` are specified as a single string, "
-                "it must be the path to a text file with a file list."
+            assert os.path.isfile(file_list), (
+                "If the `file_list` arguments is provided, it must point to a valid "
+                "path to a text file with a file list."
             )
-            with open(file_keys, "r", encoding="utf-8") as f:
+            with open(file_list, "r", encoding="utf-8") as f:
                 file_keys = f.read().splitlines()
 
         # Convert the file keys to a list of file paths with glob
+        assert file_keys is not None  # For the linter's sake
         self.file_paths = []
         if isinstance(file_keys, str):
             file_keys = [file_keys]

src/spine/io/core/read/hdf5.py

@@ -1,7 +1,7 @@
 """Contains a reader class dedicated to loading data from HDF5 files."""
 
 from dataclasses import fields
-from typing import Any, Dict, List, Optional
+from typing import Any, Dict, List, Optional, Union
 from warnings import warn
 
 import h5py
@@ -34,7 +34,8 @@ class HDF5Reader(ReaderBase):
 
     def __init__(
         self,
-        file_keys: List[str],
+        file_keys: Optional[Union[str, List[str]]] = None,
+        file_list: Optional[str] = None,
         limit_num_files: Optional[int] = None,
         max_print_files: int = 10,
         n_entry: Optional[int] = None,
@@ -53,8 +54,10 @@ def __init__(
 
         Parameters
         ----------
-        file_keys : List[str]
-            List of paths to the HDF5 files to be read
+        file_keys : Union[str, List[str]], optional
+            Path or list of paths to the HDF5 files to be read
+        file_list : str, optional
+            Path to a text file containing a list of file paths to be read
         limit_num_files : Optional[int], optional
             Integer limiting number of files to be taken per data directory
         max_print_files : int, default 10
@@ -86,7 +89,7 @@ def __init__(
             If `True`, allows missing entries in the entry or event list
         """
         # Process the list of files
-        self.process_file_paths(file_keys, limit_num_files, max_print_files)
+        self.process_file_paths(file_keys, file_list, limit_num_files, max_print_files)
 
         # If an entry list is requested based on run/subrun/event ID, create map
         if run_event_list is not None or skip_run_event_list is not None:

src/spine/io/core/read/larcv.py

@@ -1,6 +1,6 @@
 """Contains a reader class dedicated to loading data from LArCV files."""
 
-from typing import Any, Dict, List, Optional
+from typing import Any, Dict, List, Optional, Union
 
 import numpy as np
 
@@ -35,8 +35,9 @@ class LArCVReader(ReaderBase):
 
     def __init__(
         self,
-        file_keys: List[str],
-        tree_keys: List[str],
+        file_keys: Optional[Union[str, List[str]]] = None,
+        file_list: Optional[str] = None,
+        tree_keys: Optional[List[str]] = None,
         limit_num_files: Optional[int] = None,
         max_print_files: int = 10,
         n_entry: Optional[int] = None,
@@ -53,8 +54,10 @@ def __init__(
 
         Parameters
         ----------
-        file_keys : list
-            List of paths to the HDF5 files to be read
+        file_keys : Union[str, List[str]], optional
+            Path or list of paths to the HDF5 files to be read
+        file_list : str, optional
+            Path to a text file containing a list of file paths to be read
         tree_keys : List[str]
             List of data keys to load from the LArCV files
         limit_num_files : Optional[int], optional
@@ -82,7 +85,10 @@ def __init__(
             If `True`, allows missing entries in the entry or event list
         """
         # Process the file_paths
-        self.process_file_paths(file_keys, limit_num_files, max_print_files)
+        self.process_file_paths(file_keys, file_list, limit_num_files, max_print_files)
+        assert (
+            tree_keys is not None and len(tree_keys) > 0
+        ), "No input `tree_keys` provided, abort."
 
         # If an entry list is requested based on run/subrun/event ID, create map
         if run_event_list is not None or skip_run_event_list is not None: