From 2e7a45548d461f47665e50480abd0e34e73ee369 Mon Sep 17 00:00:00 2001
From: Nikolay Akhmetov <nick.akhmetov@gmail.com>
Date: Fri, 19 Jul 2024 14:25:03 -0400
Subject: [PATCH] update logic and tests now that `from_dict` works as expected

---
 .../builders/epic_builders.py                 | 29 ++++++++++---------
 test/test_builders.py                         | 17 +++++------
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/src/portal_visualization/builders/epic_builders.py b/src/portal_visualization/builders/epic_builders.py
index 5b981bc..b86ab75 100644
--- a/src/portal_visualization/builders/epic_builders.py
+++ b/src/portal_visualization/builders/epic_builders.py
@@ -9,41 +9,42 @@
 class EPICConfBuilder(ABC):
     def __init__(self, base_conf: ConfCells, epic_uuid) -> None:
 
-        if base_conf.conf is None:
+        conf, cells = base_conf
+
+        if conf is None:
             raise ValueError("ConfCells object must have a conf attribute")
 
-        self._is_plural = isinstance(base_conf.conf, list)
+        self._is_plural = isinstance(conf, list)
 
         if self._is_plural:
             self._base_conf = [
-                VitessceConfig.from_dict(conf) for conf in base_conf.conf
+                VitessceConfig.from_dict(conf) for conf in conf
             ]
         else:
             self._base_conf: VitessceConfig = VitessceConfig.from_dict(base_conf.conf)
 
-        # TODO: from_dict does not copy over requestInit options for dataset files,
-        # the function needs to be extended upstream to handle this.
-
         self._epic_uuid = epic_uuid
         pass
 
     def get_conf_cells(self):
         self.apply()
+        if (self._is_plural):
+            return get_conf_cells([conf.to_dict() for conf in self._base_conf])
         return get_conf_cells(self._base_conf)
 
+    def apply(self):
+        if self._is_plural:
+            for conf in self._base_conf:
+                self._apply(conf)
+        else:
+            self._apply(self._base_conf)
+
     @abstractmethod
-    def apply(self):  # pragma: no cover
+    def _apply(self, conf):  # pragma: no cover
         pass
 
 
 class SegmentationMaskBuilder(EPICConfBuilder):
-    def apply(self):
-        if self._is_plural:
-            for conf in self._base_conf:
-                self._apply(conf)
-            return
-        # Only expecting one dataset at this point
-        # dataset = datasets[0]
 
     def _apply(self, conf):
         datasets = conf.get_datasets()
diff --git a/test/test_builders.py b/test/test_builders.py
index 0ce26a6..0842b8c 100644
--- a/test/test_builders.py
+++ b/test/test_builders.py
@@ -176,20 +176,19 @@ def test_entity_to_vitessce_conf(entity_path, mocker):
     epic_builder = get_epic_builder(entity["uuid"])
     assert epic_builder is not None
 
-    if (conf is None):
+    if conf is None:
         with pytest.raises(ValueError):
             epic_builder(ConfCells(conf, cells), entity["uuid"]).get_conf_cells()
         return
 
-    built_epic_conf, _ = epic_builder(ConfCells(conf, cells), entity["uuid"]).get_conf_cells()
-    # Since the `from_dict` function fails to copy over the `requestInit`,
-    # the following assertion will fail. Once this is implemented upstream,
-    # the following assertion can be uncommented:
-    # assert json.dumps(built_epic_conf, indent=2, sort_keys=True) == json.dumps(
-    #     conf, indent=2, sort_keys=True
-    # )
-    # For now we just check that the builder is not None
+    built_epic_conf, _ = epic_builder(
+        ConfCells(conf, cells), entity["uuid"]
+    ).get_conf_cells()
+
     assert built_epic_conf is not None
+    assert json.dumps(built_epic_conf, indent=2, sort_keys=True) == json.dumps(
+        conf, indent=2, sort_keys=True
+    )
 
 
 @pytest.mark.parametrize("entity_path", bad_entity_paths, ids=lambda path: path.name)