DI restructuring of CellpyCell: injectable core + instrument factory (#520)

.issueflows/04-designs-and-guides/cellpycell-di-restructuring.md

@@ -0,0 +1,59 @@
+# ADR: dependency-injection restructuring of CellpyCell (#520, V2-09 tail)
+
+**Status:** accepted (2026-07-17) · **Issue:** [#520](https://github.com/jepegit/cellpy/issues/520) · **Epic:** #402 (V2-09)
+
+## Context
+
+`CellpyCell.__init__` historically built everything itself. After the
+mechanical extractions (capacity curves #509, exporters #518, split/drop
+#519), the class still *constructs* three collaborator kinds inline:
+
+1. **The core seam** — `OldCellpyCellCore` (legacy bridge) or, under the
+   `native_schema` flag (#511), `NativeCellpyCellCore`. The core owns the
+   `Data` object and runs the step/summary engine.
+2. **The instrument factory** — `register_instrument_readers()` builds
+   `ds.generate_default_factory()`; `set_instrument()` then creates
+   `self.loader_class` from it.
+3. **Config-derived options** — reader options are copied from `prms` at
+   init (`self.sep`, `self.ensure_step_table`, data dirs, …); units come
+   from `get_cellpy_units(cellpy_units)`.
+
+Only (1) and (2) are *behavioural collaborators* worth injecting: tests and
+embedders need to substitute them (fake engine, fake loader registry)
+without monkeypatching. (3) is plain data already parameterizable.
+
+## Decision
+
+**Own vs receive:**
+
+| Collaborator | `__init__` policy |
+|---|---|
+| core seam | **receive** via `core=` (default: built from the `native_schema` flag as today). An injected `core` wins; the flag then only documents intent. |
+| instrument factory | **receive** via `instrument_factory=` (default: `ds.generate_default_factory()`; built lazily in `register_instrument_readers()` only when none was injected). |
+| config snapshot, units, headers | **own** — plain data, already overridable via existing parameters (`cellpy_units=`, prms). Full config inversion is a non-goal here. |
+
+**Wiring of the extracted modules** (capacity_curves / exporters.tabular /
+slicing): stays as instance-first functions plus thin delegates. No service
+locator, no registry — the delegate methods *are* the wiring, and subclass
+dispatch works because cross-calls go through the instance.
+
+**`register_instrument_readers()`** keeps its public name but becomes
+idempotent-with-injection: it only builds the default factory when
+`self.instrument_factory` is `None`. Calling it after injecting a factory is
+a no-op instead of silently discarding the injected one.
+
+## Non-goals
+
+- Splitting `CellpyCell` into multiple classes (that is the Phase-3 /
+  native-runtime discussion, not V2-09).
+- Inverting `prms` / config access.
+- Changing `set_instrument()` semantics or the loader plug-in story (#210).
+- Removing the near-dead `_cap_mod_*` pair — decided here: **keep** them as
+  moved (#518) until a deprecation sweep; they are test-pinned and harmless.
+
+## Acceptance
+
+- `tests/test_slim.py` (core-seam acceptance) stays green throughout.
+- Defaults produce byte-identical behaviour (full suite green).
+- New pins: injecting a custom core is used by the pipeline; injecting a
+  custom instrument factory is used by `set_instrument`.

HISTORY.md

@@ -2,6 +2,13 @@
 
 ## [Unreleased]
 
+* Dependency-injection tail of V2-09 (#520): `CellpyCell(core=...,
+  instrument_factory=...)` — the core seam and the loader registry are now
+  constructor-injectable (defaults unchanged);
+  `register_instrument_readers()` keeps an injected factory instead of
+  silently rebuilding. ADR:
+  `.issueflows/04-designs-and-guides/cellpycell-di-restructuring.md`.
+
 * Split/drop-cycle helpers extracted from `cellreader.py` into
   `cellpy.readers.slicing` (#519, V2-09 follow-up): `split`, `split_many`,
   `drop_from`/`drop_to`, `from_cycle`/`to_cycle`, `drop_edges`,

cellpy/readers/cellreader.py

@@ -212,6 +212,8 @@ def __init__(
         output_units=None,
         debug=False,
         native_schema=False,
+        core=None,
+        instrument_factory=None,
     ):
         """
         Args:
@@ -226,6 +228,13 @@ def __init__(
             cellpy_units (dict): sent to cellpy.parameters.internal_settings.get_cellpy_units
             output_units (dict): sent to cellpy.parameters.internal_settings.get_default_output_units
             debug (bool): set to True if you want to see debug messages.
+            core (CellpyCellCore): injected core seam (issue #520, DI). When
+                given it is used as-is (it owns the ``Data`` object and runs
+                the step/summary engine); when None the default is built from
+                the ``native_schema`` flag.
+            instrument_factory (InstrumentFactory): injected loader registry
+                (issue #520, DI). When None,
+                ``register_instrument_readers()`` builds the default factory.
             native_schema (bool): opt-in feature flag (issue #511, V2-11).
                 When True, frames are kept in native cellpy-core column names
                 and the polars engine runs directly (no legacy rename
@@ -250,9 +259,12 @@ def __init__(
         # initialize() creates the cellpy ``ds.Data`` it expects (the data
         # property reads/writes ``self.core._data``). Under the ``native_schema``
         # opt-in (#511) the legacy bridge is replaced by the rename-free
-        # pandas<->polars adapter.
+        # pandas<->polars adapter. An injected ``core`` (#520, DI) wins over
+        # both defaults.
         self.native_schema = bool(native_schema)
-        if self.native_schema:
+        if core is not None:
+            self.core = core
+        elif self.native_schema:
             from cellpy.readers.native_core import NativeCellpyCellCore
 
             self.core = NativeCellpyCellCore(initialize=False, debug=debug)
@@ -318,7 +330,7 @@ def __init__(
         self.headers_normal = headers_normal
         self.headers_summary = headers_summary
         self.headers_step_table = headers_step_table
-        self.instrument_factory = None
+        self.instrument_factory = instrument_factory  # injected (#520) or None
         self.register_instrument_readers()
         self.set_instrument()
         # - units used by cellpy
@@ -592,9 +604,14 @@ def __register_external_readers(self):
         return
 
     def register_instrument_readers(self):
-        """Register instrument readers."""
+        """Register instrument readers.
 
-        self.instrument_factory = ds.generate_default_factory()
+        Builds the default factory only when none is set — an injected
+        ``instrument_factory`` (#520, DI) is kept as-is. Set
+        ``self.instrument_factory = None`` first to force a rebuild.
+        """
+        if self.instrument_factory is None:
+            self.instrument_factory = ds.generate_default_factory()
         # instruments = find_all_instruments()
         # for instrument_id, instrument in instruments.items():
         #     self.instrument_factory.register_builder(instrument_id, instrument)

tests/test_slim.py

@@ -107,3 +107,45 @@ def test_make_summary_selector_kwargs_deprecated(cpi, parameters):
         cpi.make_summary(selector_type="non-cv")
     assert cpi.data.summary is not None
     assert not cpi.data.summary.empty
+
+
+# --- dependency injection (#520, V2-09 tail; see the ADR in
+# --- .issueflows/04-designs-and-guides/cellpycell-di-restructuring.md) ------
+def test_injected_core_is_used_by_pipeline(parameters):
+    """An injected core seam owns the Data object and runs the pipeline."""
+    from cellpy import cellreader
+
+    core = OldCellpyCellCore(initialize=False)
+    cpi = cellreader.CellpyCell(core=core)
+    assert cpi.core is core
+
+    cpi.set_instrument("arbin_res")
+    cpi.from_raw(parameters.res_file_path)
+    cpi.make_step_table()
+
+    assert cpi.core is core
+    assert core._data is cpi.data
+    assert not cpi.data.steps.empty
+
+
+def test_injected_instrument_factory_is_used():
+    """An injected factory is used by set_instrument and never rebuilt."""
+    from cellpy import cellreader
+    from cellpy.readers import data_structures as ds
+
+    factory = ds.generate_default_factory()
+    calls = []
+    original_create = factory.create
+
+    def counting_create(instrument, **kwargs):
+        calls.append(instrument)
+        return original_create(instrument, **kwargs)
+
+    factory.create = counting_create
+    cpi = cellreader.CellpyCell(instrument_factory=factory)
+    assert cpi.instrument_factory is factory
+    assert calls  # __init__ -> set_instrument went through the injected factory
+
+    # register_instrument_readers keeps the injected factory (idempotent)
+    cpi.register_instrument_readers()
+    assert cpi.instrument_factory is factory