Incorporate maintainer feedback for experiment tests

- Fix Pyright type errors breaking CI

- Switch to tmp_path for test isolation

- Plug Windows file locks with try/finally

- Remove redundant irregular timestamp test

Author: BitForge95

tests/create_experiment.py

@@ -5,9 +5,6 @@
 
 from .create_sequence_data import _generate_sequence_data
 
-# Temporary directory for test execution
-EXPERIMENT_ROOT = Path("tests/experiment")
-
 DEFAULT_CONFIG = {
     "device_0": {
         "sampling_rate": 1.0,
@@ -33,6 +30,7 @@ def get_default_config():
 
 @contextmanager
 def create_experiment(
+    tmp_path,
     n_devices=2,
     devices_kwargs=None,
 ):
@@ -41,14 +39,18 @@ def create_experiment(
 
     devices_kwargs = [default_params | kwargs for kwargs in devices_kwargs]
 
+    assert len(devices_kwargs) == n_devices, "wrong experiment creation"
+
     try:
-        EXPERIMENT_ROOT.mkdir(parents=True, exist_ok=True)
+        tmp_path.mkdir(parents=True, exist_ok=True)
 
         for device_id, device_kwargs in enumerate(devices_kwargs):
-            device_path = EXPERIMENT_ROOT / f"device_{device_id}"
-            _generate_sequence_data(str(device_path), **device_kwargs)  # pyright: ignore
+            device_path = tmp_path / f"device_{device_id}"
+            _generate_sequence_data(
+                str(device_path), **device_kwargs
+            )  # pyright: ignore
 
-        yield EXPERIMENT_ROOT
+        yield tmp_path
     finally:
-        if EXPERIMENT_ROOT.exists():
-            shutil.rmtree(EXPERIMENT_ROOT)
+        if tmp_path.exists():
+            shutil.rmtree(tmp_path)

tests/create_sequence_data.py

@@ -20,6 +20,9 @@ def _generate_sequence_data(
     irregular=False,
 ):
     """Generates synthetic sequence data folders for testing interpolator logic."""
+    from pathlib import Path
+
+    sequence_root = Path(sequence_root)
     sequence_root.mkdir(parents=True, exist_ok=True)
     (sequence_root / "meta").mkdir(parents=True, exist_ok=True)
 
@@ -87,6 +90,7 @@ def create_sequence_data(
     t_end=10.0,
     sampling_rate=10.0,
     contain_nans=False,
+    start_time=0.0,
 ):
     """Context manager for temporary sequence data creation and cleanup."""
     try:
@@ -98,6 +102,7 @@ def create_sequence_data(
             t_end=t_end,
             sampling_rate=sampling_rate,
             contain_nans=contain_nans,
+            start_time=start_time,
         )
     finally:
         if SEQUENCE_ROOT.exists():
@@ -113,4 +118,7 @@ def sequence_data_and_interpolator(data_kwargs=None, interp_kwargs=None):
         from experanto.interpolators import Interpolator
 
         seq_interp = Interpolator.create(str(SEQUENCE_ROOT), **interp_kwargs)
-        yield timestamps, data, shifts, seq_interp
+        try:
+            yield timestamps, data, shifts, seq_interp
+        finally:
+            seq_interp.close()

tests/test_experiment.py

@@ -9,7 +9,7 @@
 from .create_experiment import create_experiment, get_default_config
 
 
-class TestInterpolator(Interpolator):
+class DummyInterpolator(Interpolator):
     """Small concrete interpolator used for testing Experiment routing logic."""
 
     def __init__(self):
@@ -22,7 +22,7 @@ def __init__(self):
 @pytest.fixture
 def mock_interpolator():
     """Shared interpolator instance to isolate Experiment logic from interpolation math."""
-    return TestInterpolator()
+    return DummyInterpolator()
 
 
 def test_experiment_initialization_and_device_loading(tmp_path, mock_interpolator):
@@ -60,17 +60,18 @@ def test_experiment_interpolate_routing(tmp_path, mock_interpolator):
 
     # Bulk routing (device=None)
     res_dict = exp.interpolate(test_times, device=None)
-    assert isinstance(res, dict)
-    assert isinstance(res["screen"], np.ndarray)
+    assert isinstance(res_dict, dict)
+    assert isinstance(res_dict["screen"], np.ndarray)
     np.testing.assert_array_equal(res_dict["screen"], np.array([1, 2, 3]))
 
 
 @pytest.mark.parametrize(
     "device_name, start_t, end_t", [("device_0", 0.0, 10.0), ("device_1", 0.0, 20.0)]
 )
-def test_get_valid_range_all_devices(device_name, start_t, end_t):
+def test_get_valid_range_all_devices(tmp_path, device_name, start_t, end_t):
     """Integration test for valid_interval propagation from disk to object."""
     with create_experiment(
+        tmp_path,
         devices_kwargs=[{"t_end": 10.0}, {"t_end": 20.0}],
     ) as experiment_path:
         experiment = Experiment(
@@ -83,8 +84,8 @@ def test_get_valid_range_all_devices(device_name, start_t, end_t):
         assert isinstance(valid_range, tuple)
 
 
-def test_get_valid_range_raises_for_invalid_device():
-    with create_experiment() as experiment_path:
+def test_get_valid_range_raises_for_invalid_device(tmp_path):
+    with create_experiment(tmp_path) as experiment_path:
         experiment = Experiment(
             root_folder=str(experiment_path),
             modality_config=get_default_config(),
@@ -93,12 +94,14 @@ def test_get_valid_range_raises_for_invalid_device():
             experiment.get_valid_range("device_does_not_exist")
 
 
-def test_experiment_with_non_zero_start_time():
+def test_experiment_with_non_zero_start_time(tmp_path):
     """Test boundary conditions for data not starting at t=0."""
     start_offset, duration = 1.5, 10.0
 
     with create_experiment(
-        devices_kwargs=[{"t_end": start_offset + duration, "start_time": start_offset}]
+        tmp_path,
+        n_devices=1,
+        devices_kwargs=[{"t_end": start_offset + duration, "start_time": start_offset}],
     ) as experiment_path:
         experiment = Experiment(
             root_folder=str(experiment_path),
@@ -110,20 +113,22 @@ def test_experiment_with_non_zero_start_time():
         assert res is not None
 
 
-def test_experiment_numeric_precision_offset():
+def test_experiment_numeric_precision_offset(tmp_path):
     """Stress test using non-integer rates and offsets to catch float drift."""
     start_offset = 0.123456789
     sampling_rate = 33.3333333
     duration = 1.0
 
     with create_experiment(
+        tmp_path,
+        n_devices=1,
         devices_kwargs=[
             {
                 "start_time": start_offset,
                 "t_end": start_offset + duration,
                 "sampling_rate": sampling_rate,
             }
-        ]
+        ],
     ) as experiment_path:
         experiment = Experiment(
             root_folder=str(experiment_path),
@@ -138,28 +143,9 @@ def test_experiment_numeric_precision_offset():
         assert res is not None
 
 
-def test_experiment_irregular_timestamps():
-    """Verify interpolation stability with jittered (non-linear) time steps."""
-    with create_experiment(
-        devices_kwargs=[{"irregular": True, "sampling_rate": 10.0}]
-    ) as experiment_path:
-        experiment = Experiment(
-            root_folder=str(experiment_path),
-            modality_config=get_default_config(),
-        )
-
-        valid_range = experiment.get_valid_range("device_0")
-        mid_point = (valid_range[0] + valid_range[1]) / 2
-
-        res = experiment.interpolate(np.array([mid_point]), device="device_0")
-        assert res is not None
-        assert isinstance(res, np.ndarray)
-        assert res.shape == (1, 10)
-
-
-def test_experiment_multi_device_interpolation():
+def test_experiment_multi_device_interpolation(tmp_path):
     """Check data consistency when interpolating across multiple modalities."""
-    with create_experiment(n_devices=2) as experiment_path:
+    with create_experiment(tmp_path, n_devices=2) as experiment_path:
         exp = Experiment(
             root_folder=str(experiment_path), modality_config=get_default_config()
         )