huggingface/datasets

Consider using "Sequence" instead of "List"

Open

#5,354 opened on Dec 12, 2022

View on GitHub
 (18 comments) (0 reactions) (1 assignee)Python (18,313 stars) (2,496 forks)batch import
enhancementgood first issue

Description

Feature request

Hi, please consider using Sequence type annotation instead of List in function arguments such as in Dataset.from_parquet(). It leads to type checking errors, see below.

How to reproduce

list_of_filenames = ["foo.parquet", "bar.parquet"]
ds = Dataset.from_parquet(list_of_filenames)

Expected mypy output:

Success: no issues found

Actual mypy output:

test.py:19: error: Argument 1 to "from_parquet" of "Dataset" has incompatible type "List[str]"; expected "Union[Union[str, bytes, PathLike[Any]], List[Union[str, bytes, PathLike[Any]]]]"  [arg-type]
test.py:19: note: "List" is invariant -- see https://mypy.readthedocs.io/en/stable/common_issues.html#variance
test.py:19: note: Consider using "Sequence" instead, which is covariant

Env: mypy 0.991, Python 3.10.0, datasets 2.7.1

Contributor guide

Tech stack
python
Domain
backenddeveloper experience
Issue type
bug
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
2
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
under 1 hour
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
blocked
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
clear
Prerequisites
Python type hintsmypy basics
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
75
Research direction
This issue suggests changing the type annotation of the `from parquet` method's argument from `List` to `Sequence` to fix a mypy error. The specific file is `src/datasets/arrow dataset.py` around line 1088. The change involves importing `Sequence` from `typing` and updating the type hint. However, the issue has an assignee, so it is likely already being worked on; consider asking for clarification or checking for a linked PR before starting.