Merge pull request #19 from HugoFara/fix/blockunit-subinfo-guards

Author: zh1peng

psyflow/BlockUnit.py

@@ -265,6 +265,12 @@ def run_trial(self, func: Callable, **kwargs) -> "BlockUnit":
         **kwargs : dict
             Additional keyword arguments forwarded to ``func``.
         """
+        if self.conditions is None:
+            raise RuntimeError(
+                f"BlockUnit '{self.block_id}' has no conditions. "
+                "Call generate_conditions() before run_trial()."
+            )
+
         self.meta['block_start_time'] = core.getAbsTime()
         self.logging_block_info()
 
@@ -273,6 +279,16 @@ def run_trial(self, func: Callable, **kwargs) -> "BlockUnit":
 
         for i, cond in enumerate(self.conditions):
             result = func(self.win, self.kb, self.settings, cond, **kwargs)
+            if not isinstance(result, dict):
+                func_name = getattr(func, "__name__", None)
+                if func_name is None and hasattr(func, "func"):
+                    func_name = getattr(func.func, "__name__", None)
+                if func_name is None:
+                    func_name = type(func).__name__
+                raise TypeError(
+                    f"Trial function {func_name!r} must return a dict, "
+                    f"got {type(result).__name__!r}"
+                )
             result.update({
                 "trial_index": i,
                 "block_id": self.block_id,
@@ -403,10 +419,14 @@ def logging_block_info(self) -> None:
         """
         Log block metadata including ID, index, seed, trial count, and condition distribution.
         """
-        dist = {c: self.conditions.count(c) for c in set(self.conditions)} if self.conditions else {}
+        if self.conditions is not None and len(self.conditions) > 0:
+            conds = np.asarray(self.conditions, dtype=object)
+            dist = {c: int(np.sum(conds == c)) for c in set(self.conditions)}
+        else:
+            dist = {}
         logging.data(f"[BlockUnit] Blockid: {self.block_id}")
         logging.data(f"[BlockUnit] Blockidx: {self.block_idx}")
         logging.data(f"[BlockUnit] Blockseed: {self.seed}")
-        logging.data(f"[BlockUnit] Blocktrial-N: {len(self.conditions)}")
+        logging.data(f"[BlockUnit] Blocktrial-N: {len(self.conditions) if self.conditions is not None else 0}")
         logging.data(f"[BlockUnit] Blockdist: {dist}")
         logging.data(f"[BlockUnit] Blockconditions: {self.conditions}")

psyflow/SubInfo.py

@@ -167,7 +167,7 @@ def validate(self, responses) -> bool:
                         raise ValueError
                     if digits is not None and len(str(val)) != digits:
                         raise ValueError
-                except:
+                except Exception:
                     infoDlg = gui.Dlg()
                     infoDlg.addText(
                         self._local("invalid_input").format(field=self._local(field['name']))

tests/test_BlockUnit.py

@@ -0,0 +1,108 @@
+"""Tests for psyflow.BlockUnit."""
+
+from functools import partial
+import unittest
+from unittest.mock import MagicMock, patch
+from types import SimpleNamespace
+
+try:
+    import numpy  # noqa: F401
+    from psychopy import core, logging  # noqa: F401
+    _HAS_DEPS = True
+except ImportError:
+    _HAS_DEPS = False
+
+if _HAS_DEPS:
+    from psyflow.BlockUnit import BlockUnit
+
+
+def _make_settings(**overrides):
+    """Create a minimal settings-like object."""
+    defaults = {
+        "trials_per_block": 3,
+        "block_seed": [42],
+    }
+    defaults.update(overrides)
+    return SimpleNamespace(**defaults)
+
+
+def _make_block(**overrides):
+    """Build a BlockUnit without calling __init__ (avoids PsychoPy window)."""
+    block = BlockUnit.__new__(BlockUnit)
+    defaults = dict(
+        block_id="test",
+        block_idx=0,
+        n_trials=3,
+        settings=_make_settings(),
+        win=MagicMock(),
+        kb=MagicMock(),
+        seed=42,
+        conditions=None,
+        results=[],
+        meta={},
+        _on_start=[],
+        _on_end=[],
+    )
+    defaults.update(overrides)
+    for k, v in defaults.items():
+        setattr(block, k, v)
+    return block
+
+
+@unittest.skipUnless(_HAS_DEPS, "requires numpy and psychopy")
+class TestRunTrialGuards(unittest.TestCase):
+    """run_trial() should reject invalid state with clear errors."""
+
+    def test_conditions_none_raises_runtime_error(self):
+        block = _make_block(conditions=None)
+
+        with self.assertRaises(RuntimeError) as ctx:
+            block.run_trial(lambda win, kb, s, c: {"rt": 0.5})
+
+        self.assertIn("conditions", str(ctx.exception).lower())
+
+    def test_func_returning_none_raises_type_error(self):
+        block = _make_block(conditions=["A"])
+
+        def bad_trial_func(win, kb, settings, cond):
+            return None
+
+        with self.assertRaises(TypeError) as ctx:
+            block.run_trial(bad_trial_func)
+
+        self.assertIn("dict", str(ctx.exception).lower())
+
+    def test_partial_trial_func_raises_type_error(self):
+        block = _make_block(conditions=["A"])
+
+        def bad_trial_func(win, kb, settings, cond):
+            return None
+
+        with self.assertRaises(TypeError) as ctx:
+            block.run_trial(partial(bad_trial_func))
+
+        self.assertIn("bad_trial_func", str(ctx.exception))
+
+
+@unittest.skipUnless(_HAS_DEPS, "requires numpy and psychopy")
+class TestLoggingBlockInfo(unittest.TestCase):
+    """logging_block_info() should handle list-backed conditions."""
+
+    def test_counts_python_list_conditions(self):
+        block = _make_block(conditions=["A", "B", "A"])
+
+        with patch("psyflow.BlockUnit.logging.data") as mock_log:
+            block.logging_block_info()
+
+        messages = [call.args[0] for call in mock_log.call_args_list]
+        self.assertTrue(
+            any(
+                "Blockdist:" in msg and "'A': 2" in msg and "'B': 1" in msg
+                for msg in messages
+            ),
+            msg=f"Unexpected log messages: {messages!r}",
+        )
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/test_SubInfo.py

@@ -0,0 +1,67 @@
+"""Tests for psyflow.SubInfo."""
+
+import unittest
+from unittest.mock import MagicMock, patch
+
+try:
+    from psychopy import gui  # noqa: F401
+    _HAS_PSYCHOPY = True
+except ImportError:
+    _HAS_PSYCHOPY = False
+
+if _HAS_PSYCHOPY:
+    import psyflow.SubInfo as _subinfo_mod
+    from psyflow.SubInfo import SubInfo
+    _gui = _subinfo_mod.gui
+
+
+def _make_subinfo(**field_map_overrides):
+    """Build a SubInfo without calling __init__."""
+    info = SubInfo.__new__(SubInfo)
+    info.fields = [
+        {"name": "subject_id", "type": "int",
+         "constraints": {"min": 101, "max": 999, "digits": 3}}
+    ]
+    info.field_map = {
+        "Participant Information": "Info",
+        "registration_failed": "Failed",
+        "invalid_input": "Bad: {field}",
+    }
+    info.field_map.update(field_map_overrides)
+    info.subject_data = None
+    return info
+
+
+@unittest.skipUnless(_HAS_PSYCHOPY, "requires psychopy")
+class TestCollect(unittest.TestCase):
+    """SubInfo.collect() control-flow edge cases."""
+
+    def test_cancel_returns_none(self):
+        info = _make_subinfo()
+
+        mock_dlg = MagicMock()
+        mock_dlg.show.return_value = None
+        with patch.object(_gui, "Dlg", return_value=mock_dlg):
+            result = info.collect(exit_on_cancel=False)
+        self.assertIsNone(result)
+
+
+@unittest.skipUnless(_HAS_PSYCHOPY, "requires psychopy")
+class TestValidate(unittest.TestCase):
+    """SubInfo.validate() error handling."""
+
+    def test_keyboard_interrupt_propagates(self):
+        info = _make_subinfo()
+
+        class ExplodingStr:
+            def __int__(self):
+                raise KeyboardInterrupt("simulated Ctrl+C")
+            def __str__(self):
+                return "boom"
+
+        with self.assertRaises(KeyboardInterrupt):
+            info.validate([ExplodingStr()])
+
+
+if __name__ == "__main__":
+    unittest.main()