Repository: LSSTDESC/sacc

Issue:
add_covariance confusing
I find `add_covariance` a bit confusing. From time to time, I find myself rechecking whether `add_covariance` overwrites the current covariance or if it adds the new covariance to the previous one. I think it would be great to check if `has_covariance` is `True` and, if so, raise an Error unless an optional argument `overwrite=True` is given. 

Discussion:

Patch:
diff --git a/sacc/sacc.py b/sacc/sacc.py
index fcf963d..1cebbba 100644
--- a/sacc/sacc.py
+++ b/sacc/sacc.py
@@ -198,7 +198,7 @@ class Sacc:
         d = DataPoint(data_type, tracers, value, **tags)
         self.data.append(d)
 
-    def add_covariance(self, covariance):
+    def add_covariance(self, covariance, overwrite=False):
         """
         Once you have finished adding data points, add a covariance
         for the entire set.
@@ -208,11 +208,19 @@ class Sacc:
         covariance: array or list
             2x2 numpy array containing the covariance of the added data points
             OR a list of blocks
+        overwrite: bool
+            If True, it overwrites the stored covariance matrix with the given
+            one.
 
         Returns
         -------
         None
         """
+        if self.has_covariance() and not overwrite:
+            raise RuntimeError("This sacc file already contains a covariance"
+                               "matrix. Use overwrite=True if you want to "
+                               "replace it for the new one")
+
         if isinstance(covariance, BaseCovariance):
             cov = covariance
         else:


Test patch:
diff --git a/test/test_sacc2.py b/test/test_sacc2.py
index 771d6c6..3d79d3f 100644
--- a/test/test_sacc2.py
+++ b/test/test_sacc2.py
@@ -66,6 +66,18 @@ def get_filled_sacc():
     return s
 
 
+def test_add_covariance():
+    s = get_filled_sacc()
+    cov = np.ones((s.mean.size, s.mean.size))
+    s.add_covariance(cov)
+
+    with pytest.raises(RuntimeError):
+        s.add_covariance(cov)
+
+    s.add_covariance(0 * cov, overwrite=True)
+    assert np.all(s.covariance.covmat == 0 * cov)
+
+
 def test_quantity_warning():
     s = sacc.Sacc()
     with pytest.warns(UserWarning):