Skip to content

Raise on Dataset.rename_columns name collision with an existing column#8323

Open
roli-lpci wants to merge 1 commit into
huggingface:mainfrom
roli-lpci:fix/rename-columns-collision
Open

Raise on Dataset.rename_columns name collision with an existing column#8323
roli-lpci wants to merge 1 commit into
huggingface:mainfrom
roli-lpci:fix/rename-columns-collision

Conversation

@roli-lpci

Copy link
Copy Markdown

What

Dataset.rename_column (singular) already rejects a new column name that collides with an
existing column:

>>> ds.rename_column("foo", "bar")  # "bar" already exists
ValueError: New column name bar already in the dataset. ...

Dataset.rename_columns (plural, dict-based) has no equivalent check. Renaming a column to a
name that collides with an existing, untouched column silently produces a Dataset whose
underlying Arrow table has two columns with the same name, while dataset.features (built
from a Python dict) collapses to one entry for that name. The two become inconsistent, and in
the reproduction below the renamed column's data becomes unreachable through the normal read
paths (ds[:], to_dict(), to_pandas(), iteration, ...) with no exception raised anywhere.

from datasets import Dataset

ds = Dataset.from_dict({"foo": [1, 2, 3], "bar": [10, 20, 30], "baz": [100, 200, 300]})
ds2 = ds.rename_columns({"foo": "bar"})

ds2.column_names   # ['bar', 'bar', 'baz']  -- two Arrow columns named "bar"
ds2.features       # {'bar': ..., 'baz': ...} -- only one "bar" entry
ds2[:]             # {'bar': [10, 20, 30], 'baz': [...]}  -- foo's [1, 2, 3] is gone

Since DatasetDict.rename_columns forwards directly to Dataset.rename_columns per split,
the same corruption is reachable from DatasetDict too.

Fix

Add the same collision check rename_column already performs, generalized to the mapping
case: a new name is rejected only if it collides with a column that isn't itself being renamed
away in the same call (so swaps like {"a": "b", "b": "a"} and no-op renames like
{"a": "a"} keep working).

colliding_new_columns = set(column_mapping.values()) & (
    set(dataset.column_names) - set(column_mapping.keys())
)
if colliding_new_columns:
    raise ValueError(
        f"New column names {colliding_new_columns} already in the dataset. "
        f"Please choose column names which are not already in the dataset. "
        f"Current columns in the dataset: {dataset._data.column_names}"
    )

Test

Added to tests/test_arrow_dataset.py::BaseDatasetTest.test_rename_columns:

  • Asserts ValueError on a colliding rename ({"col_1": "col_2"} where col_2 already
    exists and isn't being renamed).
  • Asserts a swap ({"col_1": "col_2", "col_2": "col_1"}) still succeeds, to confirm the fix
    doesn't over-reject legitimate renames.

Confirmed the added assertions fail on main with AssertionError: ValueError not raised
(not a generic error) and pass with the fix (pytest tests/test_arrow_dataset.py -k rename:
2 failed on the unmodified baseline, 4 passed with the fix). A prior full run of
tests/test_arrow_dataset.py with this change: 360 passed, 61 skipped, 1 pre-existing
unrelated failure (a PyTorch-formatting test that fails identically on main because torch
isn't installed in the test env).

Compatibility

Pure Python, no new imports, no version-gated syntax. Only affects the specific case where a
rename_columns mapping's target name collides with an existing column outside the mapping;
that call previously produced silently-corrupted output, so this is a bug fix, not a behavior
users could be relying on for correct results.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant