Refactor Settings (#387)

* refactor settings

* update tests

* replace deprecated function call

* add unit tests

* docs: update Settings class with Attributes section

* change strict_stiminterval and ignore_first_ISI to bool type

efel/api.py

@@ -36,59 +36,35 @@
 import efel.pyfeatures as pyfeatures
 from efel.pyfeatures.pyfeatures import get_cpp_feature
 
-
 _settings = efel.Settings()
-_int_settings = {}
-_double_settings = {}
-_string_settings = {}
 
 
-def reset():
-    """Resets the efel to its initial state
+def set_setting(setting_name: str, new_value: int | float | str) -> None:
+    """Set a certain setting to a new value.
 
-    The user can set certain values in the efel, like the spike threshold.
-    These values are persisten. This function will reset these value to their
-    original state.
+    Args:
+        setting_name: Name of the setting to change.
+        new_value: New value for the setting.
     """
-    global _settings, _int_settings, _double_settings, _string_settings
-    _settings = efel.Settings()
-    _int_settings = {}
-    _double_settings = {}
-    _string_settings = {}
-
-    set_double_setting('spike_skipf', 0.1)
-    set_int_setting('max_spike_skip', 2)
-    set_double_setting('Threshold', _settings.threshold)
-    set_double_setting('DerivativeThreshold', _settings.derivative_threshold)
-    set_double_setting(
-        'DownDerivativeThreshold',
-        _settings.down_derivative_threshold)
-    set_double_setting('interp_step', 0.1)
-    set_double_setting('burst_factor', 1.5)
-    set_double_setting('strict_burst_factor', 2.0)
-    set_double_setting('voltage_base_start_perc', 0.9)
-    set_double_setting('voltage_base_end_perc', 1.0)
-    set_double_setting('current_base_start_perc', 0.9)
-    set_double_setting('current_base_end_perc', 1.0)
-    set_double_setting('rise_start_perc', 0.0)
-    set_double_setting('rise_end_perc', 1.0)
-    set_double_setting("initial_perc", 0.1)
-    set_double_setting("min_spike_height", 20.0)
-    set_int_setting("strict_stiminterval", 0)
-    set_double_setting("initburst_freq_threshold", 50)
-    set_double_setting("initburst_sahp_start", 5)
-    set_double_setting("initburst_sahp_end", 100)
-    set_int_setting("DerivativeWindow", 3)
-    set_str_setting("voltage_base_mode", "mean")
-    set_str_setting("current_base_mode", "mean")
-    set_double_setting("precision_threshold", 1e-10)
-    set_double_setting("sahp_start", 5.0)
-    set_int_setting("ignore_first_ISI", 1)
-    set_double_setting("impedance_max_freq", 50.0)
+    _settings.set_setting(setting_name, new_value)
 
+
+def get_settings() -> efel.Settings:
+    """Returns the current settings of eFEL."""
+    return _settings
+
+
+def reset():
+    """Resets the efel settings to their default values.
+    see :py:func:`efel.Settings`
+    """
+    global _settings
+    _settings = efel.Settings()
+    _settings.reset_to_default()
     _initialise()
 
 
+@deprecated("Use `set_setting('dependencyfile_path', location)` instead")
 def set_dependency_file_location(location: str | Path) -> None:
     """Sets the location of the Dependency file.
 
@@ -103,10 +79,7 @@ def set_dependency_file_location(location: str | Path) -> None:
     Raises:
         FileNotFoundError: If the path to the dependency file doesn't exist.
     """
-    location = Path(location)
-    if not location.exists():
-        raise FileNotFoundError(f"Path to dependency file {location} doesn't exist")
-    _settings.dependencyfile_path = str(location)
+    set_setting('dependencyfile_path', str(location))
 
 
 def get_dependency_file_location() -> str:
@@ -118,17 +91,19 @@ def get_dependency_file_location() -> str:
     return _settings.dependencyfile_path
 
 
+@deprecated("Use `set_setting('Threshold', new_threshold)` instead")
 def set_threshold(new_threshold: float) -> None:
     """Set the spike detection threshold in the eFEL, default -20.0
 
     Args:
         new_threshold: The new spike detection threshold value (in the same units
                        as the traces, e.g. mV).
     """
-    _settings.threshold = new_threshold
-    set_double_setting('Threshold', _settings.threshold)
+    set_setting('Threshold', new_threshold)
 
 
+@deprecated("Use `set_setting('DerivativeThreshold', "
+            "new_derivative_threshold)` instead")
 def set_derivative_threshold(new_derivative_threshold: float) -> None:
     """Set the threshold for the derivative for detecting the spike onset.
 
@@ -139,8 +114,7 @@ def set_derivative_threshold(new_derivative_threshold: float) -> None:
         new_derivative_threshold: The new derivative threshold value (in the same units
                                   as the traces, e.g. mV/ms).
     """
-    _settings.derivative_threshold = new_derivative_threshold
-    set_double_setting('DerivativeThreshold', _settings.derivative_threshold)
+    set_setting('DerivativeThreshold', new_derivative_threshold)
 
 
 def get_feature_names() -> list[str]:
@@ -239,34 +213,38 @@ def _initialise() -> None:
     # flush the GErrorString from previous runs by calling getgError()
     cppcore.getgError()
 
-    # First set some settings that are used by the feature extraction
-
-    for setting_name, int_setting in list(_int_settings.items()):
-        cppcore.setFeatureInt(setting_name, [int_setting])
-
-    for setting_name, double_setting in list(_double_settings.items()):
-        if isinstance(double_setting, list):
-            cppcore.setFeatureDouble(setting_name, double_setting)
-        else:
-            cppcore.setFeatureDouble(setting_name, [double_setting])
-
-    for setting_name, str_setting in list(_string_settings.items()):
-        cppcore.setFeatureString(setting_name, str_setting)
+    # Set the settings in the cppcore
+    settings_attrs = vars(_settings)
+    for setting_name, setting_value in settings_attrs.items():
+        if isinstance(setting_value, bool):
+            setting_value = int(setting_value)
+        if isinstance(setting_value, int):
+            cppcore.setFeatureInt(setting_name, [setting_value])
+        elif isinstance(setting_value, float):
+            if isinstance(setting_value, list):
+                cppcore.setFeatureDouble(setting_name, setting_value)
+            else:
+                cppcore.setFeatureDouble(setting_name, [setting_value])
+        elif isinstance(setting_value, str):
+            cppcore.setFeatureString(setting_name, setting_value)
 
 
+@deprecated("Use `set_setting()` instead")
 def set_int_setting(setting_name: str, new_value: int) -> None:
-    """Set a certain integer setting to a new value"""
-    _int_settings[setting_name] = new_value
+    """Set a certain integer setting to a new value."""
+    set_setting(setting_name, new_value)
 
 
+@deprecated("Use `set_setting()` instead")
 def set_double_setting(setting_name: str, new_value: float) -> None:
-    """Set a certain double setting to a new value"""
-    _double_settings[setting_name] = new_value
+    """Set a certain double setting to a new value."""
+    set_setting(setting_name, new_value)
 
 
+@deprecated("Use `set_setting()` instead")
 def set_str_setting(setting_name: str, new_value: str) -> None:
-    """Set a certain string setting to a new value"""
-    _string_settings[setting_name] = new_value
+    """Set a certain string setting to a new value."""
+    set_setting(setting_name, new_value)
 
 
 @overload

efel/settings.py

@@ -19,16 +19,124 @@
  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 """
 
+from dataclasses import dataclass, fields, asdict
 from pathlib import Path
+from typing import Union
+from typing_extensions import deprecated
+import logging
 
+logger = logging.getLogger(__name__)
 
+
+@dataclass
 class Settings:
-    """FEL settings class"""
-
-    def __init__(self):
-        self.threshold = -20.0
-        self.derivative_threshold = 10.0
-        self.down_derivative_threshold = -12.0
-        self.dependencyfile_path = str(
-            Path(__file__).parent.absolute() / "DependencyV5.txt"
-        )
+    """eFEL settings class.
+
+    Attributes:
+        Threshold (float): Spike detection threshold (default: -20.0).
+        DerivativeThreshold (float): Threshold value for derivative calculations
+        (default: 10.0).
+        DownDerivativeThreshold (float): Threshold value for downward derivative
+        calculations (default: -12.0).
+        dependencyfile_path (str): Path to the dependency file
+        (default: 'DependencyV5.txt').
+        spike_skipf (float): Fraction of spikes to skip (default: 0.1).
+        max_spike_skip (int): Maximum number of spikes to skip (default: 2).
+        interp_step (float): Interpolation step (default: 0.1).
+        burst_factor (float): Burst factor (default: 1.5).
+        strict_burst_factor (float): Strict burst factor (default: 2.0).
+        voltage_base_start_perc (float): Voltage base start percentage (default: 0.9).
+        voltage_base_end_perc (float): Voltage base end percentage (default: 1.0).
+        current_base_start_perc (float): Current base start percentage (default: 0.9).
+        current_base_end_perc (float): Current base end percentage (default: 1.0).
+        rise_start_perc (float): Rise start percentage (default: 0.0).
+        rise_end_perc (float): Rise end percentage (default: 1.0).
+        initial_perc (float): Initial percentage (default: 0.1).
+        min_spike_height (float): Minimum spike height (default: 20.0).
+        strict_stiminterval (bool): Strict stimulus interval (default: False).
+        initburst_freq_threshold (int): Initial burst frequency threshold
+        (default: 50)
+        initburst_sahp_start (int): Initial burst SAHP start (default: 5).
+        initburst_sahp_end (int): Initial burst SAHP end (default: 100).
+        DerivativeWindow (int): Derivative window (default: 3).
+        voltage_base_mode (str): Voltage base mode (default: "mean").
+        current_base_mode (str): Current base mode (default: "mean").
+        precision_threshold (float): Precision threshold (default: 1e-10).
+        sahp_start (float): SAHP start (default: 5.0).
+        ignore_first_ISI (bool): Ignore first ISI (default: True).
+        impedance_max_freq (float): Impedance maximum frequency (default: 50.0).
+    """
+
+    Threshold: float = -20.0
+    DerivativeThreshold: float = 10.0
+    DownDerivativeThreshold: float = -12.0
+    dependencyfile_path: str = str(
+        Path(__file__).parent.absolute() / "DependencyV5.txt"
+    )
+
+    spike_skipf: float = 0.1
+    max_spike_skip: int = 2
+    interp_step: float = 0.1
+    burst_factor: float = 1.5
+    strict_burst_factor: float = 2.0
+    voltage_base_start_perc: float = 0.9
+    voltage_base_end_perc: float = 1.0
+    current_base_start_perc: float = 0.9
+    current_base_end_perc: float = 1.0
+    rise_start_perc: float = 0.0
+    rise_end_perc: float = 1.0
+    initial_perc: float = 0.1
+    min_spike_height: float = 20.0
+    strict_stiminterval: bool = False
+    initburst_freq_threshold: int = 50
+    initburst_sahp_start: int = 5
+    initburst_sahp_end: int = 100
+    DerivativeWindow: int = 3
+    voltage_base_mode: str = "mean"
+    current_base_mode: str = "mean"
+    precision_threshold: float = 1e-10
+    sahp_start: float = 5.0
+    ignore_first_ISI: bool = True
+    impedance_max_freq: float = 50.0
+
+    def set_setting(self,
+                    setting_name: str,
+                    new_value: Union[int, float, str, bool]) -> None:
+        """Set a certain setting to a new value.
+
+        Args:
+            setting_name (str): Name of the setting to be modified.
+            new_value (Union[int, float, str, bool]): New value for the setting.
+
+        Raises:
+            ValueError: If the value is of the wrong type.
+            FileNotFoundError: If the path to the dependency file does not exist
+            (for 'dependencyfile_path' setting).
+        """
+        if hasattr(self, setting_name):
+            expected_types = {f.name: f.type for f in fields(self)}
+            expected_type = expected_types.get(setting_name)
+            if expected_type and not isinstance(new_value, expected_type):
+                raise ValueError(f"Invalid value for setting '{setting_name}'. "
+                                 f"Expected type: {expected_type.__name__}.")
+        else:
+            logger.warning("Setting '%s' not found in settings. "
+                           "Adding it as a new setting.", setting_name)
+
+        if setting_name == "dependencyfile_path":
+            path = Path(str(new_value))
+            if not path.exists():
+                raise FileNotFoundError(f"Path to dependency file {new_value}"
+                                        "doesn't exist")
+
+        setattr(self, setting_name, new_value)
+
+    def reset_to_default(self):
+        """Reset settings to their default values"""
+        default_settings = Settings()
+        for field in default_settings.__dataclass_fields__:
+            setattr(self, field, getattr(default_settings, field))
+
+    def __str__(self):
+        attributes = asdict(self)
+        return '\n'.join([f"{key}: {value}" for key, value in attributes.items()])

examples/neo/load_nwb.ipynb

@@ -152,8 +152,8 @@
     }
    ],
    "source": [
-    "efel.set_int_setting(\"ignore_first_ISI\", 0) # Don't ignore the first spike\n",
-    "efel.set_double_setting(\"strict_burst_factor\", 4.0) # The burst detection can be fine-tuned by changing the setting strict_burst_factor. Default value is 2.0.\n",
+    "efel.set_setting(\"ignore_first_ISI\", False) # Don't ignore the first spike\n",
+    "efel.set_setting(\"strict_burst_factor\", 4.0) # The burst detection can be fine-tuned by changing the setting strict_burst_factor. Default value is 2.0.\n",
     "feature_values = efel.get_feature_values([trace], ['spikes_per_burst', 'strict_burst_number', 'strict_burst_mean_freq', 'peak_time', 'AP_height', 'peak_indices', 'burst_begin_indices', 'burst_end_indices'])[0]\n",
     "feature_values = {feature_name: list(values) for feature_name, values in feature_values.items()}\n",
     "feature_values"