deprecation warning for loc for tsdframe. disallow metadata names tha…

…t overlap with data column names for tsdframe

pynapple/core/metadata_class.py

@@ -117,10 +117,16 @@ def _raise_invalid_metadata_column_name(self, name):
                 f"Metadata name '{name}' overlaps with an existing attribute, and cannot be accessed as an attribute or key. Use 'get_info()' to access metadata."
             )
         elif hasattr(self, "columns") and name in self.columns:
-            # existing non-metadata attribute
-            warnings.warn(
-                f"Metadata name '{name}' overlaps with an existing property, and cannot be accessed as an attribute or key. Use 'get_info()' to access metadata."
-            )
+            if self.nap_class == "TsdFrame":
+                # special exception for TsdFrame columns
+                raise ValueError(
+                    f"Invalid metadata name '{name}'. Metadata name must differ from {list(self.columns)} column names!"
+                )
+            else:
+                # existing non-metadata attribute
+                warnings.warn(
+                    f"Metadata name '{name}' overlaps with an existing property, and cannot be accessed as an attribute or key. Use 'get_info()' to access metadata."
+                )
         # elif name in self.metadata_columns:
         #     # warnings for metadata that already exists
         #     warnings.warn(f"Overwriting existing metadata column '{name}'.")

pynapple/core/time_series.py

@@ -949,6 +949,11 @@ def __init__(
 
     @property
     def loc(self):
+        # add deprecation warning
+        warnings.warn(
+            "'loc' will be deprecated in a future version. Use bracket indexing instead.",
+            DeprecationWarning,
+        )
         return _TsdFrameSliceHelper(self)
 
     def __repr__(self):
@@ -1086,13 +1091,11 @@ def __getitem__(self, key, *args, **kwargs):
             and all([isinstance(k, str) for k in key])
         ):
             if all(k in self.columns for k in key):
-                return self.loc[key]
+                with warnings.catch_warnings(action="ignore"):
+                    # ignore deprecated warning for loc
+                    return self.loc[key]
             else:
-                # if all(k in self.metadata_columns for k in key):
                 return _MetadataMixin.__getitem__(self, key)
-                # else:
-                # return self.loc[key]
-
         else:
             if isinstance(key, pd.Series) and key.index.equals(self.columns):
                 # if indexing with a pd.Series from metadata, transform it to tuple with slice(None) in first position

tests/test_metadata.py

@@ -433,21 +433,23 @@ def test_tsdframe_metadata_slicing(tsdframe_meta):
 
 
 @pytest.mark.parametrize(
-    "name, attr_exp, set_exp, set_attr_exp, set_key_exp, get_attr_exp, get_key_exp",
+    "name, attr_exp, set_exp, set_attr_exp, set_key_exp, get_exp, get_attr_exp, get_key_exp",
     [
-        # pre-computed rate metadata
+        # existing data column
         (
             "a",
             # not attribute
             pytest.raises(AssertionError),
-            # warn with set_info
-            pytest.warns(UserWarning, match="overlaps with an existing"),
-            # warn with setattr
-            pytest.warns(UserWarning, match="overlaps with an existing"),
-            # cannot be set as key
+            # error with set_info
+            pytest.raises(ValueError, match="Invalid metadata name"),
+            # error with setattr
+            pytest.raises(ValueError, match="Invalid metadata name"),
+            # shape mismatch with setitem
             pytest.raises(ValueError),
-            # attribute should match metadata
-            does_not_raise(),
+            # key error with get_info
+            pytest.raises(KeyError),
+            # attribute should raise error
+            pytest.raises(AttributeError),
             # key should not match metadata
             pytest.raises(ValueError),
         ),
@@ -461,6 +463,8 @@ def test_tsdframe_metadata_slicing(tsdframe_meta):
             pytest.raises(AttributeError, match="Cannot set attribute"),
             # warn when set as key
             pytest.warns(UserWarning, match="overlaps with an existing"),
+            # no error with get_info
+            does_not_raise(),
             # attribute should not match metadata
             pytest.raises(AssertionError),
             # key should match metadata
@@ -477,6 +481,8 @@ def test_tsdframe_metadata_slicing(tsdframe_meta):
             does_not_raise(),
             # no warning with setitem
             does_not_raise(),
+            # no error with get_info
+            does_not_raise(),
             # attr should match metadata
             does_not_raise(),
             # key should match metadata
@@ -490,6 +496,7 @@ def test_tsdframe_metadata_overlapping_names(
     attr_exp,
     set_exp,
     set_attr_exp,
+    get_exp,
     set_key_exp,
     get_attr_exp,
     get_key_exp,
@@ -507,7 +514,8 @@ def test_tsdframe_metadata_overlapping_names(
     with set_key_exp:
         tsdframe_meta[name] = np.ones(4)
     # retrieve with get_info
-    assert np.all(tsdframe_meta.get_info(name) == np.ones(4))
+    with get_exp:
+        assert np.all(tsdframe_meta.get_info(name) == np.ones(4))
     # make sure it doesn't access metadata if its an existing attribute or key
     with get_attr_exp:
         assert np.all(getattr(tsdframe_meta, name) == np.ones(4))
@@ -527,7 +535,8 @@ def tsgroup_meta():
             1: nap.Ts(t=np.arange(0, 200, 0.5), time_units="s"),
             2: nap.Ts(t=np.arange(0, 300, 0.2), time_units="s"),
             3: nap.Ts(t=np.arange(0, 400, 1), time_units="s"),
-        }
+        },
+        metadata={"label": [1, 2, 3, 4]},
     )
 
 
@@ -562,6 +571,20 @@ def tsgroup_meta():
             # get key is metadata
             does_not_raise(),
         ),
+        # existing metdata
+        (
+            "label",
+            # no warning with set_info
+            does_not_raise(),
+            # no warning with setattr
+            does_not_raise(),
+            # no warning with setitem
+            does_not_raise(),
+            # attr should match metadata
+            does_not_raise(),
+            # key should match metadata
+            does_not_raise(),
+        ),
     ],
 )
 def test_tsgroup_metadata_overlapping_names(

tests/test_time_series.py

@@ -1,6 +1,7 @@
 """Tests of time series for `pynapple` package."""
 
 from numbers import Number
+import warnings
 
 import pickle
 import numpy as np
@@ -1335,6 +1336,16 @@ def test_convolve_keep_columns(self, tsdframe):
         assert isinstance(tsd2, nap.TsdFrame)
         np.testing.assert_array_equal(tsd2.columns, tsdframe.columns)
 
+    def test_deprecation_warning(self, tsdframe):
+        columns = tsdframe.columns
+        # warning using loc
+        with pytest.warns(DeprecationWarning):
+            tsdframe.loc[columns[0]]
+        if isinstance(columns[0], str):
+            # suppressed warning with getitem, which implicitly uses loc
+            with warnings.catch_warnings(action="error"):
+                tsdframe[columns[0]]
+
 
 ####################################################
 # Test for ts