code-review feedback

apis/python/src/tiledbsoma/io/registration/signatures.py

@@ -1,6 +1,6 @@
 import json
 from dataclasses import dataclass
-from typing import Dict, Optional, Tuple
+from typing import Dict, Optional
 
 import anndata as ad
 import pandas as pd
@@ -246,23 +246,23 @@ def from_soma_experiment(cls, uri: str, measurement_name: str = "RNA") -> Self:
             )
 
     @classmethod
-    def compatible(cls, signatures: Dict[str, Self]) -> Tuple[bool, Optional[str]]:
+    def check_compatible(cls, signatures: Dict[str, Self]) -> None:
         """
         Determines if any number of signatures from SOMA experiment or AnnData/H5AD will be safe
-        from schema-incompatibility at ingestion time. On success, the second argument is None; on
-        failure, the second argument can be used for reporting to the user.
+        from schema-incompatibility at ingestion time. On failure, a ``ValueError`` is raised
+        with user-suitable cause details.
         """
         if len(signatures) < 2:
-            return (True, None)
+            return
         items = list(signatures.items())
         namea, siga = items[0]
         for nameb, sigb in items[1:]:
-            if not siga.compatibleWith(sigb):
-                msg = f"Incompatible signatures {namea!r}, {nameb!r}:\n{siga.toJSON()}\n{sigb.toJSON()}"
-                return (False, msg)
-        return (True, None)
+            if not siga._compatible_with(sigb):
+                raise ValueError(
+                    f"Incompatible signatures {namea!r}, {nameb!r}:\n{siga.toJSON()}\n{sigb.toJSON()}"
+                )
 
-    def compatibleWith(self, other: Self) -> bool:
+    def _compatible_with(self, other: Self) -> bool:
         """
         Pairwise helper method for ``compatible``. Reasons for incompatibility are currently advised
         to be handled a level up by simply showing the user the failed signature pair.

apis/python/tests/test_registration_signatures.py

@@ -47,9 +47,7 @@ def test_signature_serdes(canned_h5ad_file, canned_anndata):
 def test_compatible(canned_anndata):
 
     # Check that zero inputs result in zero incompatibility
-    ok, msg = signatures.Signature.compatible({})
-    assert ok
-    assert msg is None
+    signatures.Signature.check_compatible({})
 
     sig1 = signatures.Signature.from_anndata(canned_anndata)
 
@@ -59,32 +57,23 @@ def test_compatible(canned_anndata):
     sig2 = signatures.Signature.from_soma_experiment(uri)
 
     # Check that single inputs result in zero incompatibility
-    ok, msg = signatures.Signature.compatible({"anndata": sig1})
-    assert ok
-    assert msg is None
-
-    ok, msg = signatures.Signature.compatible({"experiment": sig2})
-    assert ok
-    assert msg is None
+    signatures.Signature.check_compatible({"anndata": sig1})  # no throw
+    signatures.Signature.check_compatible({"experiment": sig2})  # no throw
 
     # Check that AnnData/H5AD is compatible with itself; likewise with SOMA Experiment
-    ok, msg = signatures.Signature.compatible({"anndata": sig1, "same": sig1})
-    assert ok
-    assert msg is None
-
-    ok, msg = signatures.Signature.compatible({"experiment": sig2, "same": sig2})
-    assert ok
-    assert msg is None
+    signatures.Signature.check_compatible({"anndata": sig1, "same": sig1})  # no throw
+    signatures.Signature.check_compatible(
+        {"experiment": sig2, "same": sig2}
+    )  # no throw
 
     # Check compatibility of identical AnnData / SOMA experiment.
-    ok, msg = signatures.Signature.compatible({"anndata": sig1, "experiment": sig2})
-    assert ok
-    assert msg is None
+    signatures.Signature.check_compatible(
+        {"anndata": sig1, "experiment": sig2}
+    )  # no throw
 
     # Check incompatibility of modified AnnData
     adata3 = canned_anndata
     del adata3.obs["groups"]
     sig3 = signatures.Signature.from_anndata(adata3)
-    ok, msg = signatures.Signature.compatible({"orig": sig1, "anndata3": sig3})
-    assert not ok
-    assert msg is not None
+    with pytest.raises(ValueError):
+        signatures.Signature.check_compatible({"orig": sig1, "anndata3": sig3})