fix: silence list_instruments probe warnings (#786)

.issueflows/03-solved-issues/issue786_original.md

@@ -0,0 +1,49 @@
+# Issue #786: list_instruments() still logs loader-probe warnings on every call
+
+Source: https://github.com/jepegit/cellpy/issues/786
+
+## Original issue text
+
+## Problem
+`cellpy.readers.data_structures.instrument_configurations()` is the right way to
+discover loaders (it's what `print_instruments` uses), but for an app it has two
+rough edges:
+
+1. **It logs a `WARNING` per non-loader module on every call** — `config_declarations`,
+   `contract`, `hooks`, `declarations`, `registry`, `testing`, and `custom`
+   ("Missing instrument definition file"). A GUI calling it at startup has to
+   bracket it with a log-level bump to keep the console clean.
+2. **The result isn't display-ready** — ids like `maccor_txt` / `pec_csv` need a
+   human label, and the raw file **suffix/extension** isn't returned alongside the
+   models, so apps keep their own label + extension map.
+
+## Suggestion
+A quiet, app-facing helper, e.g.:
+
+```python
+cellpy.list_instruments()
+# -> [{"id": "maccor_txt", "label": "Maccor (text)",
+#      "models": ["one","two","three", ...], "suffixes": [".txt"]}, ...]
+```
+
+that (a) does not emit warnings for skipped/non-loader modules, and (b) includes a
+human label and the raw suffix(es). This would let apps build an instrument picker
++ ingestion form directly from cellpy.
+
+
+---
+*Found while building [cellpy-simple-gui](https://github.com/cellpy/cellpy-simple-gui) on cellpy 2.1.0.post1. Full write-up with all items: [CELLPY_PAINPOINTS.md](https://github.com/cellpy/cellpy-simple-gui/blob/main/CELLPY_PAINPOINTS.md).*
+
+## Comments (curated summary)
+
+- **Additional tasks**:
+  - Make probe/discovery skips quiet: calling `cellpy.list_instruments()` (or `instrument_configurations()`) must not emit a `WARNING` per skipped non-loader module on every call (repro still fails on 2.1.1.post1).
+- **Clarifications / constraints**:
+  - The app-facing shape (`id` / `label` / `models` / `suffixes`) from the first #786 landing is **done** — do not rework labels/suffixes.
+  - Warnings currently hit the **root** logger (`WARNING:root:`), so silencing only `logging.getLogger("cellpy")` is ineffective.
+  - Preferred fixes (either OK): log expected discovery skips (no `DataLoader`, or `custom` with no definition file) at `DEBUG`; and/or have `list_instruments()` swallow probe failures so the public helper is quiet by contract.
+  - Remaining pain points (metadata read, per-instrument schema, figure theming) stay in #791 / related issues — out of scope here.
+- **Superseded / retracted**:
+  - Original item (2) “result isn't display-ready” — solved by the shipped `list_instruments()` helper; this reopen is **warnings-only**.
+
+_Note: this section is an interpretive summary of the comment thread, not a verbatim dump. Source comments: 1, last comment by @jepegit on 2026-07-29._

.issueflows/03-solved-issues/issue786_plan.md

@@ -0,0 +1,68 @@
+# Issue #786 — plan (warnings-only reopen)
+
+## Goal
+
+Make `cellpy.list_instruments()` (and ideally discovery via `instrument_configurations()`) quiet for **expected** loader-probe skips, so apps can call it at startup without bracketing the root logger. Keep the existing `id` / `label` / `models` / `suffixes` API unchanged.
+
+## Constraints
+
+- Warnings-only reopen: do **not** redesign labels/suffixes (already shipped in #796).
+- Out of scope: #799 / #800 / #801 / other #791 items.
+- Preserve behaviour of real loaders that fail for environmental reasons when used through normal load paths; only discovery noise should change.
+- Public surface already documented in `docs/getting_started/agents.md` — update only if the quiet contract needs an explicit note.
+
+### Prior art
+
+- `cellpy.list_instruments()` in [`cellpy/readers/data_structures.py`](cellpy/readers/data_structures.py) — already raises `logging.getLogger("cellpy")` to `ERROR` during the scan; **misses** bare `logging.warning(...)` calls that go to the **root** logger.
+- `InstrumentFactory.create_all()` in the same module — on create failure logs `logging.warning(f"Could not create loader for {key}: {e}")` except `local_instrument` (debug). This is the noise source.
+- Module already has `logger = logging.getLogger(__name__)` at top of `data_structures.py` — unused by `create_all`.
+- Tests: `test_list_instruments_shape_and_known_entry` / `test_list_instruments_is_quiet` in [`tests/test_instrument_registering.py`](tests/test_instrument_registering.py). The quiet test only asserts **Python `warnings`**, not **logging WARNING** — so it currently passes while the bug remains.
+- Toolbox / graph: nothing relevant for logging quieting.
+
+## Approach
+
+1. **Classify expected discovery skips in `InstrumentFactory.create_all`**
+   - If create fails because the module has no `DataLoader`, or `custom` raises “Missing instrument definition file…”, log at **DEBUG** via the module `logger` (not bare `logging.warning`).
+   - Other failures (e.g. missing ODBC for a real SQL loader) stay at **WARNING**, but also via the module logger so they are filterable under `cellpy.readers.data_structures` rather than `root`.
+
+2. **Make `list_instruments()` quiet by contract**
+   - Add `create_all(..., quiet=False)`; `list_instruments()` calls `create_all(quiet=True)` so *all* probe failures during that scan are DEBUG (including env-missing real loaders).
+   - Drop or shrink the ineffective `getLogger("cellpy").setLevel(ERROR)` wrapper once the above is in place (keep `warnings.catch_warnings` only if something still emits Python warnings).
+
+3. **Harden the test**
+   - Replace/extend `test_list_instruments_is_quiet` to use `caplog` at WARNING on the root logger **and** `cellpy.readers.data_structures`, asserting no “Could not create loader” records when calling `list_instruments()`.
+   - Keep the shape/label test as-is.
+   - Mark the quiet regression `@pytest.mark.essential` if Tier-1 should guard it (small, fast).
+
+4. **Docs / HISTORY** — one Unreleased bullet that #786 warnings reopen is fixed (close step); optional one-liner in agents.md that `list_instruments()` is quiet at WARNING+.
+
+## Files to touch
+
+| Path | Change |
+|------|--------|
+| `cellpy/readers/data_structures.py` | `create_all(quiet=…)`; expected skips → DEBUG; use module logger; `list_instruments` uses `quiet=True` |
+| `tests/test_instrument_registering.py` | Caplog-based quiet assertion (and essential marker if agreed) |
+| `HISTORY.md` | Unreleased fix bullet (at close) |
+| `docs/getting_started/agents.md` | Optional one-line quiet-contract note |
+
+## Test strategy
+
+```bash
+uv run pytest tests/test_instrument_registering.py -q
+uv run pytest -m essential
+```
+
+Manual smoke (matches issue repro):
+
+```python
+import logging, cellpy
+logging.basicConfig(level=logging.INFO)
+cellpy.list_instruments()  # no WARNING:root Could not create loader ...
+```
+
+## Open questions
+
+1. **Should `instrument_configurations()` / `print_instruments` also go quiet for expected non-loader skips?**  
+   **Recommended: yes** for the “no DataLoader / missing custom def” class (DEBUG), so `print_instruments` stops spam too; keep WARNING for unexpected real-loader failures unless `quiet=True`.
+2. **Essential marker on the quiet test?**  
+   **Recommended: yes** — tiny, guards the reopen regression that shipped incompletely once already.

.issueflows/03-solved-issues/issue786_status.md

@@ -0,0 +1,22 @@
+# Issue #786 — status
+
+- [x] Done
+
+## What's done
+
+- Plan accepted; build on `cursor/786-list-instruments-warnings-5e1f`.
+- `InstrumentFactory.create_all(quiet=...)`: expected discovery skips → DEBUG on module logger; unexpected failures → WARNING on module logger (not root).
+- `list_instruments()` uses `create_all(quiet=True)` (removed ineffective `cellpy` logger bump).
+- Caplog quiet regression + `@pytest.mark.essential`; agents.md quiet-contract note.
+- Verified: `list_instruments()` emits 0 "Could not create loader" WARNINGs; `instrument_configurations()` only warns for real env failures (e.g. missing ODBC).
+- HISTORY promoted to `[2.1.1.post3]`; planned release tag `v2.1.1.post3` on `master` after merge.
+- PR: https://github.com/jepegit/cellpy/pull/807 (#807)
+
+## Remaining work
+
+- None.
+
+## Release
+
+- Planned tag: `v2.1.1.post3` (git-tag derived; `post` bump from `v2.1.1.post2`)
+- Cut from `master` via `gh release create v2.1.1.post3 --target master --generate-notes` after squash-merge.

HISTORY.md

@@ -2,6 +2,15 @@
 
 ## [Unreleased]
 
+## [2.1.1.post3] - 2026-07-30
+
+Post-release of 2.1.1 — silence loader-discovery WARNING spam for apps.
+
+* `list_instruments()` is quiet by contract: expected loader-probe skips
+  (no `DataLoader` / missing custom def) log at DEBUG on the module logger;
+  the scan uses `create_all(quiet=True)` so apps no longer see
+  `WARNING:root: Could not create loader …` on every call. (#786)
+
 ## [2.1.1.post2] - 2026-07-30
 
 Post-release of 2.1.1 — collected summary facet y-axis controls for apps.

cellpy/readers/data_structures.py

@@ -851,10 +851,24 @@ def get_registered_kwargs(self):
     def get_registered_builder(self, key):
         return self._builders.get(key, None)
 
-    def create_all(self, **kwargs):
+    @staticmethod
+    def _is_expected_discovery_skip(key: str, exc: BaseException) -> bool:
+        """True for probe failures that are normal during module discovery."""
+        if key == "local_instrument":
+            return True
+        msg = str(exc)
+        if "has no attribute 'DataLoader'" in msg or 'has no attribute "DataLoader"' in msg:
+            return True
+        if "Missing instrument definition file" in msg:
+            return True
+        return False
+
+    def create_all(self, quiet: bool = False, **kwargs):
         """Create all the instrument loader modules.
 
         Args:
+            quiet: if True, log every create failure at DEBUG (used by
+                :func:`list_instruments` so apps get a silent listing).
             **kwargs: sent to the initializer of the loader class.
 
         Returns:
@@ -876,10 +890,11 @@ def create_all(self, **kwargs):
 
                 loaders[key] = models
             except Exception as e:
-                if key == "local_instrument":
-                    logging.debug(f"Could not create loader for {key}: {e}")
+                message = f"Could not create loader for {key}: {e}"
+                if quiet or self._is_expected_discovery_skip(key, e):
+                    logger.debug(message)
                 else:
-                    logging.warning(f"Could not create loader for {key}: {e}")
+                    logger.warning(message)
         return loaders
 
     @staticmethod
@@ -1077,19 +1092,13 @@ def list_instruments() -> List[Dict[str, Any]]:
         [{"id": "maccor_txt", "label": "Maccor (text)",
           "models": ["default", "ZERO", ...], "suffixes": [".txt"]}, ...]
     """
-    cellpy_logger = logging.getLogger("cellpy")
-    previous_level = cellpy_logger.level
-    cellpy_logger.setLevel(logging.ERROR)
-    try:
-        with warnings.catch_warnings():
-            warnings.simplefilter("ignore")
-            factory = InstrumentFactory()
-            for instrument, settings in find_all_instruments().items():
-                if instrument not in LOADERS_NOT_READY_FOR_PROD:
-                    factory.register_builder(instrument, settings)
-            loaders = factory.create_all()
-    finally:
-        cellpy_logger.setLevel(previous_level)
+    with warnings.catch_warnings():
+        warnings.simplefilter("ignore")
+        factory = InstrumentFactory()
+        for instrument, settings in find_all_instruments().items():
+            if instrument not in LOADERS_NOT_READY_FOR_PROD:
+                factory.register_builder(instrument, settings)
+        loaders = factory.create_all(quiet=True)
 
     listing: List[Dict[str, Any]] = []
     for loader_id, models in loaders.items():

docs/getting_started/agents.md

@@ -134,8 +134,9 @@ Suggested layering:
       `collection.plot(y_ranges={"coulombic_efficiency": [0, 110]})`
       (Plotly). `share_y=True` / `match_axes=True` restores a shared scale.
     - **Instrument picker for free.** `cellpy.list_instruments()` returns
-      `[{"id", "label", "models", "suffixes"}, ...]` (quiet — no per-module
-      warnings), ready to drive an import form.
+      `[{"id", "label", "models", "suffixes"}, ...]` and is quiet by contract
+      (probe/discovery skips stay at DEBUG — no `WARNING` spam on the root
+      logger), ready to drive an import form.
     - **Keep the console quiet.** cellpy logs through the `cellpy` logger; raise
       its level in an app you want silent:
       `logging.getLogger("cellpy").setLevel(logging.ERROR)`. Suppress one-off

tests/test_instrument_registering.py

@@ -209,13 +209,26 @@ def test_list_instruments_shape_and_known_entry():
     assert by_id["pec_csv"]["label"] == "PEC (CSV)"
 
 
-def test_list_instruments_is_quiet():
-    """No per-non-loader-module warnings escape (the whole point, #786)."""
+@pytest.mark.essential
+def test_list_instruments_is_quiet(caplog):
+    """No per-non-loader-module warnings escape (the whole point, #786).
+
+    Probe failures historically used bare ``logging.warning`` (root logger);
+    silencing only ``logging.getLogger("cellpy")`` was not enough. Assert on
+    logging WARNING records, not just Python ``warnings``.
+    """
     import warnings
 
     import cellpy
 
     with warnings.catch_warnings(record=True) as caught:
         warnings.simplefilter("always")
-        cellpy.list_instruments()
+        with caplog.at_level(logging.WARNING):
+            cellpy.list_instruments()
     assert not caught, [str(w.message) for w in caught]
+    probe_noise = [
+        r
+        for r in caplog.records
+        if r.levelno >= logging.WARNING and "Could not create loader" in r.getMessage()
+    ]
+    assert not probe_noise, [r.getMessage() for r in probe_noise]