diff --git a/pynapple/core/metadata_class.py b/pynapple/core/metadata_class.py index 9e17b774..a7fb1a7b 100644 --- a/pynapple/core/metadata_class.py +++ b/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}'.") diff --git a/pynapple/core/time_series.py b/pynapple/core/time_series.py index dadee775..8c0a8fc5 100644 --- a/pynapple/core/time_series.py +++ b/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 diff --git a/tests/test_metadata.py b/tests/test_metadata.py index 28a6f926..84570576 100644 --- a/tests/test_metadata.py +++ b/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( diff --git a/tests/test_time_series.py b/tests/test_time_series.py index ac8956c3..f25f479c 100755 --- a/tests/test_time_series.py +++ b/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