Remove pandas inplace and 3.1 deprecations

.github/workflows/ci.yaml

@@ -30,10 +30,18 @@ jobs:
 
   build:
     needs: determine_version
+    # Keeps the check names "build (<platform>, <python>)"; only the minimum-pandas job adds a suffix.
+    name: build (${{ matrix.platform }}, ${{ matrix.python-version }}${{ matrix.pandas != 'latest' && format(', pandas {0}', matrix.pandas) || '' }})
     strategy:
       matrix:
         platform: [ubuntu-latest]
         python-version: ${{fromJson(needs.determine_version.outputs.matrix)}}
+        # "latest": the newest pandas the Python version allows. The extra job tests the floor in pyproject.toml.
+        pandas: [latest]
+        include:
+          - platform: ubuntu-latest
+            python-version: "3.10"
+            pandas: "2.2.3"
 
     runs-on: ${{ matrix.platform }}
 
@@ -58,6 +66,10 @@ jobs:
       - name: Install dependencies
         run: uv pip install -e ".[docs,test]"
 
+      - name: Install the minimum supported pandas
+        if: matrix.pandas != 'latest'
+        run: uv pip install "pandas==${{ matrix.pandas }}"
+
       # Run spec tests without coverage for non Python 3.10
       - name: Run spec_test
         env:

hed/models/df_util.py

@@ -118,6 +118,8 @@ def process_def_expands(
 def sort_dataframe_by_onsets(df):
     """Sort a dataframe by the onset column.
 
+    The sort is stable: rows with equal onsets keep their order in the file.
+
     Parameters:
         df(pd.Dataframe): Dataframe to sort.
 
@@ -128,8 +130,7 @@ def sort_dataframe_by_onsets(df):
         # Create a copy and sort by onsets as floats(if needed), but continue to keep the string version.
         df_copy = df.copy()
         df_copy["_temp_onset_sort"] = pd.to_numeric(df_copy["onset"], errors="coerce")
-        df_copy.sort_values(by="_temp_onset_sort", inplace=True)
-        df_copy.drop(columns=["_temp_onset_sort"], inplace=True)
+        df_copy = df_copy.sort_values(by="_temp_onset_sort", kind="stable").drop(columns=["_temp_onset_sort"])
 
         return df_copy
     return df
@@ -276,7 +277,7 @@ def split_delay_tags(series, hed_schema, onsets):
         insert_index = split_df["original_index"].index.max() + 1
         split_df.loc[insert_index] = {"HED": str(group), "onset": onset_mod, "original_index": i}
     split_df = sort_dataframe_by_onsets(split_df)
-    split_df.reset_index(drop=True, inplace=True)
+    split_df = split_df.reset_index(drop=True)
 
     split_df = filter_series_by_onset(split_df, split_df.onset)
     return split_df

hed/schema/schema_io/hed_id_util.py

@@ -364,4 +364,4 @@ def merge_dfs(dest_df, source_df):
         if not match_index.empty:
             for col in source_df.columns:
                 if col not in save_df1_columns:
-                    dest_df.at[match_index[0], col] = row[col]
+                    dest_df.loc[match_index[0], col] = row[col]  # .at cannot add a column (pandas 3.1)

hed/tools/analysis/key_map.py

@@ -93,7 +93,7 @@ def make_template(self, additional_cols=None, show_counts=True):
             df[additional_cols] = "n/a"
         if show_counts:
             df.insert(0, "key_counts", self._get_counts())
-            df.sort_values(by=["key_counts"], inplace=True, ignore_index=True, ascending=False)
+            df = df.sort_values(by=["key_counts"], ignore_index=True, ascending=False)
         return df
 
     def _get_counts(self):
@@ -172,11 +172,16 @@ def _remap(self, df):
         # Copy all the map_dict data into merged_df as new columns, merging on the map_dict_index number of both
         remapped_df = pd.merge(
             merged_df, self.col_map, left_on="key_value", right_index=True, suffixes=("", "_new"), how="left"
-        ).fillna("n/a")
+        )
 
-        # Override the original columns with our newly calculated ones
+        # Override the original columns with our newly calculated ones; a row without a mapping gets "n/a".
+        # pandas 3.1 deprecates filling a string into a numeric column, so such a column becomes object first
+        # (the dtype the fill used to produce).
         for col in self.target_cols:
-            df[col] = remapped_df[col + "_new"]
+            new_col = remapped_df[col + "_new"]
+            if pd.api.types.is_numeric_dtype(new_col) and new_col.isna().any():
+                new_col = new_col.astype(object)
+            df[col] = new_col.fillna("n/a")
 
         # Finally calculate missing indices
         missing_indices = key_series.index[key_values.isna()].tolist()
@@ -185,7 +190,7 @@ def _remap(self, df):
 
     def resort(self):
         """Sort the col_map in place by the key columns."""
-        self.col_map.sort_values(by=self.key_cols, inplace=True, ignore_index=True)
+        self.col_map = self.col_map.sort_values(by=self.key_cols, ignore_index=True)
         for index, row in self.col_map.iterrows():
             key_hash = data_util.get_row_hash(row, self.key_cols)
             self.map_dict[key_hash] = index

hed/tools/util/data_util.py

@@ -1,5 +1,7 @@
 """Data handling utilities involving dataframes."""
 
+import warnings
+
 import numpy as np
 import pandas as pd
 
@@ -63,11 +65,14 @@ def delete_columns(df, column_list):
     """
 
     delete_cols = list(set(column_list).intersection(set(df)))
-    df.drop(columns=delete_cols, inplace=True)
+    for col in delete_cols:
+        del df[col]
 
 
 def delete_rows_by_column(df, value, column_list=None):
-    """Delete rows where columns have this value.
+    """Deprecated (removed in hedtools 2.0.0): delete rows where columns have this value, in place.
+
+    Use :func:`remove_rows_by_value`, which returns a new dataframe.
 
     Parameters:
         df (DataFrame):      Pandas dataframe from which to delete rows.
@@ -79,14 +84,48 @@ def delete_rows_by_column(df, value, column_list=None):
         - Deletion is done in place.
 
     """
+    warnings.warn(
+        "delete_rows_by_column is deprecated and will be removed in hedtools 2.0.0; use remove_rows_by_value, "
+        "which returns a new dataframe instead of deleting in place.",
+        DeprecationWarning,
+        stacklevel=2,
+    )
     if column_list:
         cols = list(set(column_list).intersection(set(df)))
     else:
         cols = list(df)
+    # pandas has no warning-free way to drop rows from the same object, and pandas 3.1 deprecates inplace=;
+    # silence pandas' warning so the caller sees only the one above. This shim goes in 2.0.0.
+    with warnings.catch_warnings():
+        warnings.simplefilter("ignore", DeprecationWarning)
+        for col in cols:
+            map_col = df[col].map(str) == str(value)
+            df.drop(df[map_col].index, axis=0, inplace=True)  # noqa: PD002
+
+
+def remove_rows_by_value(df, value, column_list=None):
+    """Return a dataframe without the rows where any of the columns has this value.
+
+    Parameters:
+        df (DataFrame):      Pandas dataframe to filter; it is not changed.
+        value (str):         Specified value to indicate row should be removed.
+        column_list (list):  List of columns to search for value; all columns if None or empty.
+
+    Returns:
+        DataFrame: A new dataframe holding the remaining rows, with their original index.
 
+    Notes:
+        - All values are converted to string before testing.
+
+    """
+    if column_list:
+        cols = list(set(column_list).intersection(set(df)))
+    else:
+        cols = list(df)
+    mask = pd.Series(False, index=df.index)
     for col in cols:
-        map_col = df[col].map(str) == str(value)
-        df.drop(df[map_col].index, axis=0, inplace=True)
+        mask |= df[col].map(str) == str(value)
+    return df[~mask]
 
 
 def get_eligible_values(values, values_included):

pyproject.toml

@@ -174,6 +174,7 @@ select = [
     "B",  # flake8-bugbear
     "C4", # flake8-comprehensions
     "UP", # pyupgrade
+    "PD002", # pandas-vet: inplace=True (deprecated in pandas 3.1, removed in pandas 4)
 ]
 ignore = [
     "E501",  # line too long, handled by ruff format

tests/test_deprecations.py

@@ -20,6 +20,7 @@
 from hed.schema import load_schema
 from hed.schema.hed_schema_io import from_string
 from hed.schema.schema_io.base2schema import SchemaLoader
+from hed.tools.util import data_util
 from hed.validator import SpreadsheetValidator
 from hed.validator.util import UnitValueValidator
 
@@ -54,6 +55,11 @@ def _has_parameter(func, name):
         lambda: _has_parameter(UnitValueValidator.__init__, "value_validators"),
         2,
     ),
+    (
+        "hed.tools.util.data_util.delete_rows_by_column (replaced by remove_rows_by_value)",
+        lambda: hasattr(data_util, "delete_rows_by_column"),
+        2,
+    ),
 ]
 
 
@@ -150,5 +156,23 @@ def test_no_warning_without_value_validators(self):
         self.assertFalse(any(issubclass(w.category, DeprecationWarning) for w in caught))
 
 
+class TestDeleteRowsByColumnDeprecation(unittest.TestCase):
+    """delete_rows_by_column deleted rows in place, which pandas 3.1 deprecates; remove_rows_by_value returns a copy."""
+
+    def test_warns_names_replacement_and_still_deletes_in_place(self):
+        import pandas as pd
+
+        df = pd.DataFrame({"a": ["x", "n/a", "y"], "b": ["1", "2", "n/a"]})
+        with warnings.catch_warnings(record=True) as caught:
+            warnings.simplefilter("always")
+            result = data_util.delete_rows_by_column(df, "n/a")
+        deprecations = [w for w in caught if issubclass(w.category, DeprecationWarning)]
+        self.assertEqual(len(deprecations), 1, [str(w.message) for w in deprecations])
+        self.assertIn("remove_rows_by_value", str(deprecations[0].message))
+        self.assertIn("2.0.0", str(deprecations[0].message))
+        self.assertIsNone(result)
+        self.assertListEqual(list(df.index), [0])
+
+
 if __name__ == "__main__":
     unittest.main()

tests/test_pandas3_compat.py

@@ -12,19 +12,29 @@
   6. get_attributes_from_row receiving NaN instead of a string
   7. _verify_hedid_matches receiving NaN hedId (broke .startswith())
   8. assign_hed_ids_section: float NaN is truthy, so bare `if hed_id:` skipped rows
+  9. sort_dataframe_by_onsets: the default quicksort did not keep tied onsets in file order
+ 10. pandas 3.1 deprecations (Pandas4Warning, a DeprecationWarning; pandas 4 raises): inplace=, filling a
+     string into a numeric column (KeyMap remap), adding a column through .at (merge_dfs)
 """
 
 import unittest
+import warnings
 
 import pandas as pd
 
 from hed import load_schema_version
-from hed.models.df_util import _filter_by_index_list, shrink_defs
+from hed.models.df_util import _filter_by_index_list, shrink_defs, sort_dataframe_by_onsets, split_delay_tags
 from hed.schema.hed_schema_io import from_dataframes
 from hed.schema.schema_io import df_constants as constants
 from hed.schema.schema_io.df_util import _merge_dataframes, get_attributes_from_row
-from hed.schema.schema_io.hed_id_util import _get_hedid_range, _verify_hedid_matches, assign_hed_ids_section
-from hed.tools.util.data_util import delete_columns
+from hed.schema.schema_io.hed_id_util import (
+    _get_hedid_range,
+    _verify_hedid_matches,
+    assign_hed_ids_section,
+    merge_dfs,
+)
+from hed.tools.analysis.key_map import KeyMap
+from hed.tools.util.data_util import delete_columns, get_row_hash, remove_rows_by_value
 
 hed_schema_global = load_schema_version("8.4.0")
 
@@ -447,5 +457,97 @@ def test_empty_string_id_gets_assigned(self):
         self.assertTrue(isinstance(new_id, str) and new_id.startswith("HED_"))
 
 
+# ---------------------------------------------------------------------------
+# 9. sort_dataframe_by_onsets - rows with equal onsets keep their file order
+# ---------------------------------------------------------------------------
+
+
+class TestOnsetSortStable(unittest.TestCase):
+    """Validate that sort_dataframe_by_onsets is a stable sort.
+
+    ``sort_values`` defaults to ``kind="quicksort"``, which is not stable, so rows sharing an onset could come
+    out in any order. Rows that share an onset are merged into one string at the first tied row, so the row an
+    issue is reported at depended on the sort. The fix sorts with ``kind="stable"``.
+    """
+
+    def test_tied_onsets_keep_file_order(self):
+        """Out-of-order onsets with ties of three: each tie keeps its rows in file order."""
+        onsets = [str(float(k // 3)) for k in range(12)][::-1]
+        df = pd.DataFrame({"onset": onsets, "HED": [f"Label/r{k}" for k in range(12)]})
+        result = sort_dataframe_by_onsets(df)
+        self.assertListEqual(list(result.index), [9, 10, 11, 6, 7, 8, 3, 4, 5, 0, 1, 2])
+        self.assertNotIn("_temp_onset_sort", result.columns)
+
+    def test_input_not_changed(self):
+        """The input frame keeps its rows and columns."""
+        df = pd.DataFrame({"onset": ["2.0", "1.0"], "HED": ["Red", "Blue"]})
+        sort_dataframe_by_onsets(df)
+        self.assertListEqual(list(df.index), [0, 1])
+        self.assertListEqual(list(df.columns), ["onset", "HED"])
+
+
+# ---------------------------------------------------------------------------
+# 10. inplace= - no pandas deprecation from the functions that used it
+# ---------------------------------------------------------------------------
+
+
+class TestNoPandasDeprecationWarnings(unittest.TestCase):
+    """Run each function that used ``inplace=`` with DeprecationWarning turned into an error.
+
+    pandas 3.1 deprecates ``inplace=`` with ``Pandas4Warning``, a ``DeprecationWarning``, and pandas 4 removes it.
+    Under pandas 3.1 these tests fail if a call warns again; under earlier pandas they pass either way.
+    """
+
+    def setUp(self):
+        catcher = warnings.catch_warnings()
+        catcher.__enter__()
+        self.addCleanup(catcher.__exit__, None, None, None)
+        warnings.simplefilter("error", DeprecationWarning)
+
+    def test_sort_dataframe_by_onsets(self):
+        df = pd.DataFrame({"onset": ["2.0", "1.0"], "HED": ["Red", "Blue"]})
+        self.assertListEqual(list(sort_dataframe_by_onsets(df).index), [1, 0])
+
+    def test_split_delay_tags(self):
+        series = pd.Series(["Red,(Delay/3.0 s,(Blue))", "Green"])
+        result = split_delay_tags(series, load_schema_version("8.5.0"), pd.Series([1.0, 2.0]))
+        self.assertListEqual(list(result.index), [0, 1, 2])
+        self.assertListEqual(list(result.onset), [1.0, 2.0, 4.0])
+
+    def test_key_map_make_template_and_resort(self):
+        key_map = KeyMap(["event_type"], ["code"])
+        key_map.update(pd.DataFrame({"event_type": ["b", "a", "b"], "code": ["2", "1", "2"]}))
+        template = key_map.make_template(show_counts=True)
+        self.assertListEqual(list(template["key_counts"]), [2, 1])
+        key_map.resort()
+        self.assertListEqual(list(key_map.col_map["event_type"]), ["a", "b"])
+        self.assertEqual(key_map.map_dict[get_row_hash(key_map.col_map.iloc[0], ["event_type"])], 0)
+
+    def test_delete_columns(self):
+        df = pd.DataFrame({"a": [1], "b": [2]})
+        delete_columns(df, ["a"])
+        self.assertListEqual(list(df.columns), ["b"])
+
+    def test_remove_rows_by_value(self):
+        df = pd.DataFrame({"a": ["x", "n/a"], "b": ["1", "2"]})
+        self.assertListEqual(list(remove_rows_by_value(df, "n/a").index), [0])
+
+    def test_key_map_remap_missing_key_numeric_columns(self):
+        """A missing key fills "n/a" into a numeric target column; numeric data columns are left alone."""
+        key_map = KeyMap(["k"], ["num"])
+        key_map.update(pd.DataFrame({"k": ["a", "b"], "num": [1.5, 2.0]}))
+        df_new, missing = key_map.remap(pd.DataFrame({"k": ["a", "z", "b"], "onset": [0.5, 1.0, 2.0]}))
+        self.assertListEqual(list(df_new["num"]), [1.5, "n/a", 2.0])
+        self.assertListEqual(list(df_new["onset"]), [0.5, 1.0, 2.0])
+        self.assertListEqual(missing, [1])
+
+    def test_merge_dfs_adds_column(self):
+        """merge_dfs copies a column dest_df lacks into the matching row."""
+        dest = pd.DataFrame({"rdfs:label": ["A", "B"]})
+        merge_dfs(dest, pd.DataFrame({"rdfs:label": ["B"], "extra": ["x"]}))
+        self.assertEqual(dest.loc[1, "extra"], "x")
+        self.assertTrue(pd.isna(dest.loc[0, "extra"]))
+
+
 if __name__ == "__main__":
     unittest.main()

tests/tools/util/test_data_util.py

@@ -15,6 +15,7 @@
     get_row_hash,
     get_value_dict,
     make_info_dataframe,
+    remove_rows_by_value,
     reorder_columns,
     replace_na,
     replace_values,
@@ -65,9 +66,24 @@ def test_delete_rows_by_column(self):
         data = {"Name": ["n/a", "", "tom", "alice", 0, 1], "Age": [np.nan, 10, "", "n/a", "0", "10"]}
         df1 = DataFrame(data)
         self.assertEqual(len(df1.index), 6, "The data frame should have 6 rows to start")
-        delete_rows_by_column(df1, "")
+        with self.assertWarns(DeprecationWarning):
+            delete_rows_by_column(df1, "")
         self.assertEqual(len(df1.index), 4, "The data frame should have 4 rows after deletion")
 
+    def test_remove_rows_by_value(self):
+        data = {"Name": ["n/a", "", "tom", "alice", 0, 1], "Age": [np.nan, 10, "", "n/a", "0", "10"]}
+        df1 = DataFrame(data)
+        df2 = remove_rows_by_value(df1, "")
+        self.assertEqual(len(df2.index), 4, "The returned data frame should have 4 rows")
+        self.assertListEqual(list(df2.index), [0, 3, 4, 5], "The remaining rows keep their original index")
+        self.assertEqual(len(df1.index), 6, "The original data frame should still have 6 rows")
+
+    def test_remove_rows_by_value_column_list(self):
+        data = {"Name": ["n/a", "", "tom", "alice", 0, 1], "Age": [np.nan, 10, "", "n/a", "0", "10"]}
+        df1 = DataFrame(data)
+        self.assertListEqual(list(remove_rows_by_value(df1, "n/a", ["Age", "Missing"]).index), [0, 1, 2, 4, 5])
+        self.assertListEqual(list(remove_rows_by_value(df1, "0").index), [0, 1, 2, 3, 5])
+
     def test_get_new_dataframe(self):
         df_new = get_new_dataframe(self.stern_map_path)
         self.assertIsInstance(df_new, DataFrame)