fix(spikeinterface_types): address PR review comments (ITL-459)

- Move pytest.importorskip() into each SI-dependent test so that
  test_spikeinterface_not_installed_raises_import_error runs without SI
- Fix finally block cleanup to handle ImportError when SI is genuinely absent
- Update spec: LazyModule -> try/except ImportError, v0.1.json -> register_spikeinterface_types(),
  load_extractor -> spikeinterface.core.load throughout

Author: kurodo3[bot]

superpowers/specs/2026-07-01-spikeinterface-baserecording-design.md

@@ -27,7 +27,7 @@ it. orcapod stores that JSON string and delegates all disk I/O to SI.
   return values with `BaseRecording` without errors.
 - `python_to_storage()` serializes via `recording.to_dict(recursive=True)` → JSON string.
   Raises a clear `ValueError` for in-memory recordings (`check_serializability("json") == False`).
-- `storage_to_python()` reconstructs via `spikeinterface.core.load_extractor(dict)`.
+- `storage_to_python()` reconstructs via `spikeinterface.core.load(dict)`.
 - A `SIRecordingHandler` registered in the semantic hasher that hashes the JSON string
   (phase 1 — source-path content hashing is deferred to a follow-up issue).
 - `spikeinterface` available as `pip install orcapod[spikeinterface]`; imports are lazy so
@@ -43,8 +43,11 @@ it. orcapod stores that JSON string and delegates all disk I/O to SI.
 
 New file: `src/orcapod/extension_types/spikeinterface_types.py`
 
-SI imports use `LazyModule` (same pattern as polars/pyarrow in the codebase) so that
-`import orcapod` does not fail when SpikeInterface is not installed.
+`spikeinterface_types.py` imports `BaseRecording` at module level using a `try/except ImportError`
+block that re-raises with a clear pip-install message if SI is absent. The
+`extension_types/__init__.py` wraps the import in its own `try/except ImportError` so that
+`import orcapod` never fails when SI is not installed — the SI types are silently omitted from
+`__all__` in that case.
 
 ### Optional dependency
 
@@ -57,9 +60,11 @@ spikeinterface = ["spikeinterface>=0.101"]
 
 ### Registration
 
-`LogicalSIRecording` is registered in `contexts/data/v0.1.json` alongside
-`LogicalFile`, `LogicalDirectory`, and `LogicalNumpyArray`. The `SIRecordingHandler`
-is registered in the `python_type_handler_registry` in the same context config.
+`LogicalSIRecording` and `SIRecordingHandler` are **not** registered in
+`contexts/data/v0.1.json` — doing so would break startup for users who have not installed the
+`spikeinterface` extras group. Instead, users opt in explicitly by calling
+`register_spikeinterface_types()` once at application startup. This function accepts an optional
+`DataContext`; when called with no argument it registers into the default context.
 
 ---
 
@@ -97,7 +102,7 @@ for `np.ndarray`.
 **`storage_to_python(storage_value, converter)`:**
 
 1. Parses `json.loads(storage_value)` to recover the SI dict.
-2. Calls `spikeinterface.core.load_extractor(si_dict)` and returns the result.
+2. Calls `spikeinterface.core.load(si_dict)` and returns the result.
 3. If the backing zarr/folder has been moved or deleted, SI raises naturally — no
    additional wrapping needed (same behaviour as `LogicalDirectory` with a deleted path).
 
@@ -132,7 +137,7 @@ hashing infrastructure to land first (ITL-467).
 |-----------|-----------|
 | `NumpyRecording` or any recording with `check_serializability("json") == False` | `ValueError` clarifying that lazy file-backed recordings are fine but in-memory ones are not, with save instructions |
 | `spikeinterface` not installed | `ImportError` on first use of `LogicalSIRecording`, message: `"Install spikeinterface: pip install orcapod[spikeinterface]"` |
-| Backing zarr/folder deleted after storage | SI raises `FileNotFoundError` or similar on `load_extractor()` — propagates as-is |
+| Backing zarr/folder deleted after storage | SI raises `FileNotFoundError` or similar on `load()` — propagates as-is |
 | Corrupt JSON in storage | `json.JSONDecodeError` raised with the raw value in the message |
 
 ---
@@ -171,7 +176,7 @@ so they are skipped automatically when SI is not installed.
 
 - `src/orcapod/extension_types/directory_type.py` — pattern for JSON-storage LogicalType
 - `src/orcapod/extension_types/numpy_type.py` — pattern for direct python_type binding + hash handler
-- `src/orcapod/hashing/semantic_hashing/builtin_handlers.py` — where `SIRecordingHandler` registers
-- `src/orcapod/contexts/data/v0.1.json` — context config where LogicalType + handler are wired in
+- `src/orcapod/extension_types/spikeinterface_types.py` — implementation (LogicalType, handler, registration)
+- `src/orcapod/contexts/data/v0.1.json` — context config changelog entry (LogicalType/handler NOT wired in statically)
 - SpikeInterface `BaseExtractor.to_dict()` / `check_serializability()`: `spikeinterface/core/base.py`
 - ITL-467 — database-level artifact storage (phase 2 dependency)

tests/test_extension_types/test_spikeinterface_types.py

@@ -9,7 +9,11 @@
 
 
 def test_spikeinterface_not_installed_raises_import_error(monkeypatch):
-    """Importing spikeinterface_types when SI is absent raises ImportError."""
+    """Importing spikeinterface_types when SI is absent raises ImportError.
+
+    This test runs regardless of whether spikeinterface is installed — it
+    simulates absence by blocking the import via sys.modules.
+    """
     import sys
     import importlib
 
@@ -36,22 +40,24 @@ def test_spikeinterface_not_installed_raises_import_error(monkeypatch):
             sys.modules[si_types_key] = saved_si_types
         else:
             sys.modules.pop(si_types_key, None)
-            importlib.import_module(si_types_key)
-
-
-# All tests below require spikeinterface — skip if not installed
-si = pytest.importorskip("spikeinterface", reason="spikeinterface not installed")
-import spikeinterface.core as si_core  # noqa: E402
+            # Re-import only if SI is genuinely available; if not, there is
+            # nothing to restore and the ImportError is expected.
+            try:
+                importlib.import_module(si_types_key)
+            except ImportError:
+                pass
 
 
-def _make_numpy_recording() -> si_core.NumpyRecording:
+def _make_numpy_recording():
     """Create a small in-memory NumpyRecording for use as a test source."""
+    import spikeinterface.core as si_core
     rng = np.random.default_rng(42)
     traces = rng.standard_normal((200, 4)).astype("float32")
     return si_core.NumpyRecording([traces], sampling_frequency=30_000)
 
 
 def test_logical_si_recording_importable():
+    si_core = pytest.importorskip("spikeinterface.core", reason="spikeinterface not installed")
     from orcapod.extension_types.spikeinterface_types import LogicalSIRecording
     import pyarrow as pa
 
@@ -63,6 +69,7 @@ def test_logical_si_recording_importable():
 
 def test_in_memory_recording_raises():
     """NumpyRecording (json=False) raises ValueError with clear instructions."""
+    pytest.importorskip("spikeinterface", reason="spikeinterface not installed")
     from orcapod.extension_types.spikeinterface_types import LogicalSIRecording
 
     rec = _make_numpy_recording()
@@ -78,6 +85,7 @@ def test_in_memory_recording_raises():
 
 def test_folder_recording_round_trip(tmp_path):
     """Binary-folder-backed recording round-trips through python_to_storage / storage_to_python."""
+    pytest.importorskip("spikeinterface", reason="spikeinterface not installed")
     from orcapod.extension_types.spikeinterface_types import LogicalSIRecording
 
     saved = _make_numpy_recording().save_to_folder(str(tmp_path / "rec"))
@@ -97,6 +105,7 @@ def test_folder_recording_round_trip(tmp_path):
 
 def test_zarr_recording_round_trip(tmp_path):
     """Zarr-backed recording round-trips through python_to_storage / storage_to_python."""
+    pytest.importorskip("spikeinterface", reason="spikeinterface not installed")
     from orcapod.extension_types.spikeinterface_types import LogicalSIRecording
 
     saved = _make_numpy_recording().save_to_zarr(str(tmp_path / "rec.zarr"))
@@ -120,6 +129,7 @@ def test_ephemeral_recording_round_trip(tmp_path):
     SpikeInterface re-applies the preprocessing chain lazily (using zscore
     which requires no optional dependencies and is fully deterministic).
     """
+    pytest.importorskip("spikeinterface", reason="spikeinterface not installed")
     import spikeinterface.preprocessing as spre
     from orcapod.extension_types.spikeinterface_types import LogicalSIRecording
 
@@ -143,6 +153,7 @@ def test_ephemeral_recording_round_trip(tmp_path):
 
 def test_si_recording_handler_hash_stability(tmp_path):
     """Same recording produces identical ContentHash across two calls."""
+    pytest.importorskip("spikeinterface", reason="spikeinterface not installed")
     from orcapod.extension_types.spikeinterface_types import SIRecordingHandler
     from orcapod.types import ContentHash
 
@@ -158,6 +169,7 @@ def test_si_recording_handler_hash_stability(tmp_path):
 
 def test_si_recording_handler_hash_changes_with_content(tmp_path):
     """Different recordings produce different ContentHash values."""
+    si_core = pytest.importorskip("spikeinterface.core", reason="spikeinterface not installed")
     from orcapod.extension_types.spikeinterface_types import SIRecordingHandler
 
     rng = np.random.default_rng(0)
@@ -174,6 +186,7 @@ def test_si_recording_handler_hash_changes_with_content(tmp_path):
 
 def test_si_recording_handler_in_memory_raises():
     """SIRecordingHandler raises ValueError for in-memory recordings."""
+    pytest.importorskip("spikeinterface", reason="spikeinterface not installed")
     from orcapod.extension_types.spikeinterface_types import SIRecordingHandler
 
     rec = _make_numpy_recording()
@@ -185,6 +198,7 @@ def test_si_recording_handler_in_memory_raises():
 def test_register_spikeinterface_types(tmp_path):
     """register_spikeinterface_types() wires LogicalSIRecording and SIRecordingHandler
     into the default context so they are found by type lookup."""
+    pytest.importorskip("spikeinterface", reason="spikeinterface not installed")
     from orcapod.extension_types.spikeinterface_types import register_spikeinterface_types
     from orcapod.contexts import get_default_context