addresses first major round of comments on #41

.coveragerc

@@ -0,0 +1,7 @@
+[report]
+exclude_lines =
+    # Have to re-enable the standard pragma
+    pragma: no cover
+
+    # Don't complain if tests don't hit defensive assertion code:
+    raise NotImplementedError

.github/workflows/ci.yaml

@@ -74,7 +74,6 @@ jobs:
       - name: Upload Coverage to Codecov
         uses: codecov/codecov-action@v2
         with:
-          token: ${{ secrets.CODECOV_TOKEN }} # not required for public repos
           verbose: true # optional (default = false)
       #----------------------------------------------
       #        test docs build only on PR

pgmax/fg/groups.py

@@ -16,7 +16,7 @@
 
 @dataclass(frozen=True, eq=False)
 class VariableGroup:
-    """Base class to represent a group of variables.
+    """Class to represent a group of variables.
 
     All variables in the group are assumed to have the same size. Additionally, the
     variables are indexed by a "key", and can be retrieved by direct indexing (even indexing
@@ -35,12 +35,12 @@ def __post_init__(self) -> None:
         )
 
     @typing.overload
-    def __getitem__(self, key: Hashable) -> nodes.Variable:  # pragma: no cover
-        pass
+    def __getitem__(self, key: Hashable) -> nodes.Variable:
+        """This function must be overriden by a subclass; does nothing"""
 
     @typing.overload
-    def __getitem__(self, key: List) -> List[nodes.Variable]:  # pragma: no cover
-        pass
+    def __getitem__(self, key: List) -> List[nodes.Variable]:
+        """This function must be overriden by a subclass; does nothing"""
 
     def __getitem__(self, key):
         """Given a key, retrieve the associated Variable.
@@ -84,9 +84,7 @@ def _set_keys_to_vars(self) -> Dict[Any, nodes.Variable]:
             "Please subclass the VariableGroup class and override this method"
         )
 
-    def get_vars_to_evidence(
-        self, evidence: Any
-    ) -> Dict[nodes.Variable, np.ndarray]:  # pragma: no cover
+    def get_vars_to_evidence(self, evidence: Any) -> Dict[nodes.Variable, np.ndarray]:
         """Function that turns input evidence into a dictionary mapping variables to evidence.
 
         Returns:
@@ -143,13 +141,13 @@ def __post_init__(self):
             self, "_keys_to_vars", MappingProxyType(self._set_keys_to_vars())
         )
 
-    @typing.overload  # pragma: no cover
+    @typing.overload
     def __getitem__(self, key: Hashable) -> nodes.Variable:
-        pass
+        """This function must be overriden by a subclass; does nothing"""
 
-    @typing.overload  # pragma: no cover
+    @typing.overload
     def __getitem__(self, key: List) -> List[nodes.Variable]:
-        pass
+        """This function must be overriden by a subclass; does nothing"""
 
     def __getitem__(self, key):
         """Given a key, retrieve the associated Variable from the associated VariableGroup.
@@ -355,7 +353,7 @@ def get_vars_to_evidence(
 
 @dataclass(frozen=True, eq=False)
 class FactorGroup:
-    """Base class to represent a group of factors.
+    """Class to represent a group of factors.
 
     Args:
         variable_group: either a VariableGroup or - if the elements of more than one VariableGroup
@@ -384,7 +382,7 @@ def factors(self) -> Tuple[nodes.EnumerationFactor, ...]:
 
 @dataclass(frozen=True, eq=False)
 class EnumerationFactorGroup(FactorGroup):
-    """Base class to represent a group of EnumerationFactors.
+    """Class to represent a group of EnumerationFactors.
 
     All factors in the group are assumed to have the same set of valid configurations and
     the same potential function. Note that the log potential function is assumed to be
@@ -430,7 +428,7 @@ def factors(self) -> Tuple[nodes.EnumerationFactor, ...]:
 
 @dataclass(frozen=True, eq=False)
 class PairwiseFactorGroup(FactorGroup):
-    """Base class to represent a group of EnumerationFactors where each factor connects to
+    """Class to represent a group of EnumerationFactors where each factor connects to
     two different variables.
 
     All factors in the group are assumed to be such that all possible configuration of the two

pgmax/fg/nodes.py

@@ -63,7 +63,7 @@ class EnumerationFactor:
 
     Args:
         variables: List of involved variables
-        configs: 2D array of shape (num_val_configs, num_variables)
+        configs: Array of shape (num_val_configs, num_variables)
             An array containing an explicit enumeration of all valid configurations
         factor_configs_log_potentials: Array of shape (num_val_configs,). An array containing
             the log of the potential value for every possible configuration

pytest.ini

@@ -1,3 +1,3 @@
 [pytest]
 filterwarnings =
-    ignore::DeprecationWarning
+    ignore::DeprecationWarning:flatbuffers

tests/fg/test_graph.py

@@ -1,19 +1,7 @@
-import pytest
-
 from pgmax.fg import graph, groups
 
 
 def test_onevar_graph():
     v_group = groups.GenericVariableGroup(15, tuple([0]))
     fg = graph.FactorGraph(v_group)
     assert fg._composite_variable_group[0, 0].num_states == 15
-
-
-def test_set_evidence_Error():
-    v_group = groups.GenericVariableGroup(15, tuple([0]))
-    fg = graph.FactorGraph(v_group)
-    fg.evidence_default_mode = "test"
-    with pytest.raises(NotImplementedError) as nerror:
-        fg.evidence
-
-    assert "evidence_default_mode" in str(nerror.value)

tests/fg/test_groups.py

@@ -9,12 +9,6 @@ def test_vargroup_list_idx():
     assert v_group[[0, 1, 2]][0].num_states == 15
 
 
-def test_vargroup_set_keys_to_vars():
-    with pytest.raises(NotImplementedError) as nerror:
-        groups.VariableGroup()
-    assert "Please subclass" in str(nerror.value)
-
-
 def test_composite_vargroup_valueerror():
     v_group1 = groups.GenericVariableGroup(15, tuple([0, 1, 2]))
     v_group2 = groups.GenericVariableGroup(15, tuple([0, 1, 2]))
@@ -32,10 +26,14 @@ def test_composite_vargroup_evidence():
         len(
             comp_var_group.get_vars_to_evidence(
                 [{0: np.zeros(3)}, {0: np.zeros(3)}]
-            ).keys()
+            ).values()
         )
         == 2
     )
+    for arr in comp_var_group.get_vars_to_evidence(
+        [{0: np.zeros(3)}, {0: np.zeros(3)}]
+    ).values():
+        assert (arr == np.zeros(3, dtype=float)).all()
 
 
 def test_ndvararray_evidence_error():
@@ -51,11 +49,6 @@ def test_facgroup_errors():
         groups.FactorGroup(v_group, [])
     assert "self.connected_var_keys is empty" == str(verror.value)
 
-    fac_group = groups.FactorGroup(v_group, [[(0, 0), (1, 1)]])
-    with pytest.raises(NotImplementedError) as nerror:
-        fac_group.factors
-    assert "Needs to be overriden by subclass" == str(nerror.value)
-
 
 def test_pairwisefacgroup_errors():
     v_group = groups.NDVariableArray(3, (2, 2))

tests/test_pgmax.py

@@ -365,7 +365,7 @@ def create_valid_suppression_config_arr(suppression_diameter):
     )
 
     # Run BP
-    one_step_msgs = fg.run_bp(100, 0.5)
+    one_step_msgs = fg.run_bp(1, 0.5)
     final_msgs = fg.run_bp(99, 0.5, one_step_msgs)
 
     # Test that the output messages are close to the true messages