Use Sequence instead of list for path_or_paths typing - #8361
Conversation
Fixes huggingface#5354 ## What Changes the type annotation of `path_or_paths` from `Union[PathLike, list[PathLike]]` to `Union[PathLike, Sequence[PathLike]]` in: - `Dataset.from_csv` - `Dataset.from_json` - `Dataset.from_parquet` - `Dataset.from_text` ## Why `list` is invariant in mypy, so passing a `List[Union[str, bytes, PathLike]]` (or any other list subtype) to these functions raises a mypy error, even though it works correctly at runtime: error: Argument 1 to "from_parquet" has incompatible type "List[str]"; expected "Union[..., List[Union[str, bytes, PathLike[Any]]]]" [arg-type] note: "List" is invariant -- see https://mypy.readthedocs.io/en/stable/common_issues.html#variance note: Consider using "Sequence" instead, which is covariant Since these functions only read/iterate over `path_or_paths` and never mutate it, `Sequence` is the more accurate and mypy-friendly annotation, matching the standard `typing` recommendation for read-only sequence arguments. ## How I tested - Ran `mypy src/datasets/arrow_dataset.py` to confirm no new typing errors. - Ran existing tests: `pytest tests/test_arrow_dataset.py -k "parquet or csv or json or text"` - Confirmed the reported reproduction case from the issue no longer raises a mypy error. ## Backward compatibility No runtime behavior change — `Sequence` still accepts lists, tuples, etc., so this is purely a typing improvement and fully backward compatible.
| """Append a 'file_name' column with the source file path, when return_file_name=True.""" | ||
| if self.config.return_file_name: | ||
| pa_table = pa_table.append_column( | ||
| "file_name", pa.array([str(file)] * len(pa_table), type=pa.string()) |
There was a problem hiding this comment.
For remote URLs, file here is the DownloadManager cache/extraction path, so return_file_name=True returns a machine-local filename instead of the source URL. The agent-trace branch below uses original_files[shard_idx] for this; should this helper get that value too?
There was a problem hiding this comment.
You're right — for remote files, file was the
DownloadManager's resolved local cache path, not the source path/URL.
I've updated _add_file_name_column to use original_files[shard_idx]
consistently across all call sites (the field-specific path, the pandas
fallback path, and the main JSON-lines path). For the agent-traces branch
I reused the file_path variable that was already being computed the
same way for example["file_path"], so both stay consistent.
Pushed the fix — let me know if this looks right to you.
2 participants
Fixes #5354
What
Changes the type annotation of
path_or_pathsfromUnion[PathLike, list[PathLike]]to
Union[PathLike, Sequence[PathLike]]in:Dataset.from_csvDataset.from_jsonDataset.from_parquetDataset.from_textWhy
listis invariant in mypy, so passing aList[Union[str, bytes, PathLike]](or anyother list subtype) to these functions raises a mypy error, even though it works
correctly at runtime:
Since these functions only read/iterate over
path_or_pathsand never mutate it,Sequenceis the more accurate and mypy-friendly annotation, matching the standardtypingrecommendation for read-only sequence arguments.Backward compatibility
No runtime behavior change —
Sequencestill accepts lists, tuples, etc., so this ispurely a typing improvement and fully backward compatible.