Repository: rhayes777/PyAutoFit
Pull request #959 (2024-03-04 16:38:32)

Revert "feature/concise graph"
Reverts rhayes777/PyAutoFit#957


Patch:
diff --git a/autofit/mapper/model.py b/autofit/mapper/model.py
index afe4b6aec..c6e6042a4 100644
--- a/autofit/mapper/model.py
+++ b/autofit/mapper/model.py
@@ -94,10 +94,6 @@ class AbstractModel(ModelObject):
         self.__dict__.update(state)
         self._frozen_cache = {}
 
-    def reference_count(self, child):
-        _, values = zip(*self.attribute_tuples_with_type(type(child)))
-        return values.count(child)
-
     def freeze(self):
         """
         Freeze this object.
diff --git a/autofit/mapper/prior/abstract.py b/autofit/mapper/prior/abstract.py
index c0602ea4f..35f80496e 100644
--- a/autofit/mapper/prior/abstract.py
+++ b/autofit/mapper/prior/abstract.py
@@ -190,10 +190,7 @@ class Prior(Variable, ABC, ArithmeticMixin):
     def __getattr__(self, item):
         if item in ("__setstate__", "__getstate__"):
             raise AttributeError(item)
-        try:
-            return getattr(self.message, item)
-        except AttributeError:
-            raise AttributeError(f"Attribute {item} not found in {self}")
+        return getattr(self.message, item)
 
     def __eq__(self, other):
         try:
diff --git a/autofit/mapper/prior/arithmetic/compound.py b/autofit/mapper/prior/arithmetic/compound.py
index 43d2fe668..a0aa70a53 100644
--- a/autofit/mapper/prior/arithmetic/compound.py
+++ b/autofit/mapper/prior/arithmetic/compound.py
@@ -16,7 +16,7 @@ def retrieve_name(var):
     first_name = None
     frame = inspect.currentframe()
     while frame is not None:
-        for name, value in list(frame.f_locals.items()):
+        for name, value in frame.f_locals.items():
             if var is value:
                 first_name = name
         frame = frame.f_back
diff --git a/autofit/mapper/prior_model/attribute_pair.py b/autofit/mapper/prior_model/attribute_pair.py
index 917453021..fbea896e6 100644
--- a/autofit/mapper/prior_model/attribute_pair.py
+++ b/autofit/mapper/prior_model/attribute_pair.py
@@ -34,9 +34,6 @@ class AttributeNameValue:
             return self.tuple == other
         return False
 
-    def __lt__(self, other):
-        return tuple(self) < tuple(other)
-
     def __hash__(self):
         return hash(self.tuple)
 
diff --git a/autofit/text/formatter.py b/autofit/text/formatter.py
index e86c7da89..ed55bb624 100644
--- a/autofit/text/formatter.py
+++ b/autofit/text/formatter.py
@@ -48,12 +48,6 @@ class FormatNode:
                     lines.append(f"{indent_string}{line}")
         return lines
 
-    def __getattr__(self, item):
-        try:
-            return getattr(self.value, item)
-        except AttributeError:
-            raise AttributeError(f"Attribute {item} not found in {self}")
-
 
 class TextFormatter:
     def __init__(self, line_length=90, indent=4):
diff --git a/autofit/text/representative.py b/autofit/text/representative.py
index 1c0229a47..364cfcabe 100644
--- a/autofit/text/representative.py
+++ b/autofit/text/representative.py
@@ -1,8 +1,7 @@
-from collections import Counter
-from typing import List, Iterable, Set
+from collections import defaultdict
+from typing import Dict, List, Iterable
 
 import autofit as af
-from autofit.mapper.prior.abstract import Prior
 
 
 class Representative:
@@ -39,10 +38,7 @@ class Representative:
         """
         The first object in the group represents the group.
         """
-        try:
-            return sorted(self.children)[0]
-        except TypeError:
-            return self.children[0]
+        return self.items[0][1]
 
     def __getattr__(self, item):
         return getattr(self.representative, item)
@@ -69,41 +65,18 @@ class Representative:
         -------
         A list of representatives and items that are not part of a representative.
         """
-        representatives = []
-        last_blue_print = None
-        current_items = []
+        representative_dict: Dict[tuple, list] = defaultdict(list)
+        for key, obj in items:
+            blueprint = cls.get_blueprint(obj)
+            representative_dict[blueprint].append((key, obj))
 
-        def add():
-            if len(current_items) >= minimum:
-                representative = Representative(current_items)
+        representatives = []
+        for blueprint, items in representative_dict.items():
+            if len(items) >= minimum:
+                representative = Representative(items)
                 representatives.append((representative.key, representative))
             else:
-                representatives.extend(current_items)
-
-        shared_priors = cls.shared_descendents(obj for _, obj in items)
-
-        for key, obj in sorted(items):
-            try:
-                if any(
-                    prior in shared_priors
-                    for _, prior in obj.path_instance_tuples_for_class(af.Prior)
-                ):
-                    add()
-                    current_items = [(key, obj)]
-                    last_blue_print = None
-                    continue
-            except AttributeError:
-                pass
-
-            blueprint = cls.get_blueprint(obj)
-            if blueprint == last_blue_print:
-                current_items.append((key, obj))
-            else:
-                add()
-                current_items = [(key, obj)]
-                last_blue_print = blueprint
-
-        add()
+                representatives.extend(items)
 
         return representatives
 
@@ -145,43 +118,7 @@ class Representative:
                 )
                 if path != ("id",)
             )
-            path_priors = obj.path_instance_tuples_for_class(
-                af.Prior, ignore_children=True
-            )
-            min_id = min(pp[1].id for pp in path_priors)
-            blueprint += tuple(
-                (path, prior.id - min_id, cls.get_blueprint(prior))
-                for path, prior in obj.path_instance_tuples_for_class(
-                    af.Prior, ignore_children=True
-                )
-            )
             if isinstance(obj, af.Model):
                 return blueprint + (obj.cls,)
             return blueprint
         raise ValueError(f"Cannot get blueprint for {obj} of type {type(obj)}")
-
-    @classmethod
-    def shared_descendents(cls, objects) -> Set[Prior]:
-        """
-        Find all priors which are shared by more than one object in a list of items.
-
-        Parameters
-        ----------
-        objects
-            A list of objects.
-
-        Returns
-        -------
-        A set of priors shared by more than one object.
-        """
-        counts = Counter()
-        for obj in objects:
-            try:
-                for _, prior in obj.path_instance_tuples_for_class(
-                    af.Prior, ignore_children=True
-                ):
-                    counts[prior] += 1
-            except AttributeError:
-                pass
-
-        return {prior for prior, count in counts.items() if count > 1}
diff --git a/autofit/visualise.py b/autofit/visualise.py
index bdcee82c3..33614b144 100644
--- a/autofit/visualise.py
+++ b/autofit/visualise.py
@@ -1,5 +1,6 @@
 from typing import Dict, List
 
+import networkx as nx
 from pyvis.network import Network
 import colorsys
 
@@ -13,7 +14,6 @@ from autofit.mapper.prior.log_uniform import LogUniformPrior
 from autofit.mapper.prior_model.prior_model import ModelObject
 from autofit.mapper.prior_model.prior_model import Model
 from autofit.mapper.prior_model.collection import Collection
-from autofit.text.representative import Representative
 
 
 def str_for_object(obj: ModelObject) -> str:
@@ -30,8 +30,6 @@ def str_for_object(obj: ModelObject) -> str:
     -------
     A string representation of the object.
     """
-    if isinstance(obj, Representative):
-        return str_for_object(obj.representative)
     if isinstance(obj, Collection):
         return f"{obj.id}:Collection({len(obj)})"
     if isinstance(obj, Model):
@@ -76,6 +74,36 @@ class VisualiseGraph:
         """
         self.model = model
 
+    def graph(self) -> nx.DiGraph:
+        """
+        Generate a graph of the model with just edges. Could
+        be superseded by the network method.
+        """
+        graph = nx.DiGraph()
+
+        def add_model(model):
+            model_name = str_for_object(model)
+
+            for name, prior in model.direct_prior_tuples:
+                prior_name = str_for_object(prior)
+                graph.add_edge(
+                    model_name,
+                    prior_name,
+                    label=name,
+                )
+
+            for name, child_model in model.direct_prior_model_tuples:
+                add_model(child_model)
+                graph.add_edge(
+                    model_name,
+                    str_for_object(child_model),
+                    label=name,
+                )
+
+        add_model(self.model)
+
+        return graph
+
     @cached_property
     def colours(self) -> Dict[type, str]:
         """
@@ -139,51 +167,27 @@ class VisualiseGraph:
                 **kwargs,
             )
 
-        def add_component(component):
-            model_name = str_for_object(component)
-
-            if isinstance(component, Model):
-                add_model(component)
-            elif isinstance(component, Collection):
-                add_collection(component)
-
-            for name, representative in Representative.find_representatives(
-                component.direct_prior_tuples
-            ):
-                try:
-                    prior = representative.representative
-                except AttributeError:
-                    prior = representative
-
-                prior_name = str_for_object(prior)
-                net.add_node(
-                    prior_name,
-                    shape="dot",
-                    color=self.colours[type(prior)],
-                    size=10,
-                )
-                net.add_edge(
-                    model_name,
-                    prior_name,
-                    label=name,
-                )
-
-            for name, representative in Representative.find_representatives(
-                component.direct_prior_model_tuples
-            ):
-                try:
-                    child_model = representative.representative
-                except AttributeError:
-                    child_model = representative
+        if isinstance(self.model, Model):
+            add_model(self.model, borderWidth=5)
+        else:
+            add_collection(self.model, borderWidth=5)
 
-                add_component(child_model)
-                net.add_edge(
-                    model_name,
-                    str_for_object(child_model),
-                    label=name,
-                )
+        for _, model in self.model.attribute_tuples_with_type(Model):
+            add_model(model)
+        for _, collection in self.model.attribute_tuples_with_type(
+            Collection,
+            ignore_children=False,
+        ):
+            add_collection(collection)
+        for _, prior in self.model.prior_tuples:
+            net.add_node(
+                str_for_object(prior),
+                shape="dot",
+                color=self.colours[type(prior)],
+                size=10,
+            )
 
-        add_component(self.model)
+        net.from_nx(self.graph())
 
         return net
 


Test patch:
diff --git a/test_autofit/mapper/model/derived_quantities/test_derived_quantities.py b/test_autofit/mapper/model/derived_quantities/test_derived_quantities.py
index 2344f4613..b37a3af21 100644
--- a/test_autofit/mapper/model/derived_quantities/test_derived_quantities.py
+++ b/test_autofit/mapper/model/derived_quantities/test_derived_quantities.py
@@ -149,6 +149,6 @@ def test_custom_derived_summary(samples, custom_model):
 
 Summary (3.0 sigma limits):
 
-custom        0.0000 (0.0000, 0.0000)
-fwhm          2.3548 (2.3548, 2.3548)"""
+fwhm          2.3548 (2.3548, 2.3548)
+custom        0.0000 (0.0000, 0.0000)"""
     )
diff --git a/test_autofit/mapper/test_parameterization.py b/test_autofit/mapper/test_parameterization.py
index 0dfa92c76..e74f2662a 100644
--- a/test_autofit/mapper/test_parameterization.py
+++ b/test_autofit/mapper/test_parameterization.py
@@ -66,6 +66,18 @@ def make_info_dict():
 
 
 class TestGenerateModelInfo:
+    def test_info_string(self, formatter):
+        ls = formatter.list
+
+        assert ls[0] == "one"
+        assert len(ls[1]) == 21
+        assert ls[1] == "    one             1"
+        assert ls[2] == "    two             2"
+        assert ls[3] == "    three"
+        assert ls[4] == "        four        4"
+        assert ls[5] == "three"
+        assert ls[6] == "    four            4"
+
     def test_basic(self):
         mm = af.ModelMapper()
         mm.mock_class = af.m.MockClassx2
diff --git a/test_autofit/test_visualise.py b/test_autofit/test_visualise.py
index 268b187a1..06b433166 100644
--- a/test_autofit/test_visualise.py
+++ b/test_autofit/test_visualise.py
@@ -33,6 +33,28 @@ def test_visualise(model, visualise_graph, graph_path):
     assert graph_path.exists()
 
 
+@pytest.fixture
+def graph(visualise_graph):
+    return visualise_graph.graph()
+
+
+@pytest.mark.parametrize("node", ["0:Model(Gaussian)", "3:UniformPrior(0.0, 1.0)"])
+def test_nodes(graph, node):
+    assert node in graph.nodes
+
+
+@pytest.mark.parametrize(
+    "edge",
+    [
+        ("2:Collection(2)", "0:Model(Gaussian)"),
+        ("2:Collection(2)", "1:Model(Gaussian)"),
+        ("0:Model(Gaussian)", "1:UniformPrior(0.0, 1.0)"),
+    ],
+)
+def test_edges(graph, edge):
+    assert edge in graph.edges
+
+
 @pytest.fixture
 def graph_path(output_directory):
     return output_directory / "graph.html"
diff --git a/test_autofit/text/test_concise.py b/test_autofit/text/test_concise.py
index 734c970fe..a18f55c49 100644
--- a/test_autofit/text/test_concise.py
+++ b/test_autofit/text/test_concise.py
@@ -5,7 +5,6 @@ import pytest
 
 from autofit.text.representative import Representative
 from autofit.text.text_util import result_info_from
-from autofit.visualise import VisualiseGraph
 
 
 @pytest.fixture(autouse=True)
@@ -40,36 +39,13 @@ def test_representative(collection):
     assert key == "0 - 9"
 
 
-def test_reference_count(collection):
-    assert collection.reference_count(collection[0].centre) == 1
-
-    collection[5].centre = collection[0].centre
-    assert collection.reference_count(collection[0].centre) == 2
-
-
-def test_find_representatives(collection):
-    assert len(Representative.find_representatives(collection.items())) == 1
-
-    collection[0].centre = af.UniformPrior(0.0, 1.0)
-    assert len(Representative.find_representatives(collection.items())) == 2
-
-
-def test_mid_collection_anomaly(collection):
-    collection[5].centre = af.UniformPrior(0.0, 1.0)
-    assert len(Representative.find_representatives(collection.items())) == 3
-
-
-def test_shared_prior(collection):
-    collection[5].centre = collection[0].centre
-    assert len(Representative.find_representatives(collection.items())) == 4
-
-
-def test_shared_descendents(collection):
-    assert Representative.shared_descendents(collection) == set()
-
-    shared = collection[1].centre
-    collection[0].centre = shared
-    assert Representative.shared_descendents(collection) == {shared}
+def test_get_blueprint():
+    assert Representative.get_blueprint(af.Model(af.Gaussian)) == (
+        (("centre",), (af.UniformPrior, "lower_limit = 0.0, upper_limit = 1.0")),
+        (("normalization",), (af.UniformPrior, "lower_limit = 0.0, upper_limit = 1.0")),
+        (("sigma",), (af.UniformPrior, "lower_limit = 0.0, upper_limit = 1.0")),
+        af.Gaussian,
+    )
 
 
 @pytest.fixture(name="samples")
@@ -123,12 +99,3 @@ instances
 
 """
     )
-
-
-def test_visualise(collection, output_directory):
-    collection[5].centre = collection[1].centre
-
-    output_path = output_directory / "test.html"
-    VisualiseGraph(collection).save(str(output_path))
-
-    assert output_path.exists()