Skip to content

Keep the id of List and LargeList features when visiting them - #8389

Open
LeSingh1 wants to merge 1 commit into
huggingface:mainfrom
LeSingh1:fix/features-list-id
Open

Keep the id of List and LargeList features when visiting them#8389
LeSingh1 wants to merge 1 commit into
huggingface:mainfrom
LeSingh1:fix/features-list-id

Conversation

@LeSingh1

@LeSingh1 LeSingh1 commented Aug 4, 2026

Copy link
Copy Markdown

_visit() and _visit_with_path() rebuild List / LargeList in order to recurse into the inner feature, but they don't carry id over, so it is silently reset to None. Every other feature type is passed through untouched and keeps its id.

Features.__init__ runs _visit() on every column (to convert the legacy [feature] syntax to List), so this is reachable from plain user code:

>>> Features({"a": Value("int32", id="my feature")})["a"].id
'my feature'
>>> Features({"a": List(Value("int32"), id="my feature")})["a"].id
None

id is a live, tested concept — tests/features/test_features.py already asserts it survives for Value.

This passes id through in both visitors so list features behave like the rest. Nested cases are covered too (Features({"a": {"b": List(..., id=...)}})).

Added test_visit_with_list_types_keeps_id, parametrized over List, List with length, and LargeList; all three fail on main with assert None == 'my feature' and pass with the change. Regression run across tests/features/, tests/test_arrow_writer.py, tests/test_table.py, tests/test_arrow_dataset.py and tests/test_info.py → 1150 passed, 129 skipped.

`_visit()` and `_visit_with_path()` rebuild `List` and `LargeList` in order to
recurse into the inner feature, but they did not carry `id` over, so it was
silently reset to None. Every other feature type is passed through untouched
and keeps its `id`.

`Features.__init__` runs `_visit()` on every column (to convert the legacy
`[feature]` syntax to `List`), so this was reachable from plain user code:

    >>> Features({"a": Value("int32", id="my feature")})["a"].id
    'my feature'
    >>> Features({"a": List(Value("int32"), id="my feature")})["a"].id
    None

Pass `id` through in both visitors so list features behave like the rest.
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