helmo/github-issues huggingface dataset:
Support webdataset format
State: closed | Author: lhoestq (MEMBER)

Webdataset is an efficient format for iterable datasets. It would be nice to support it in `datasets`, as discussed in https://github.com/rom1504/img2dataset/issues/234.

In particular it would be awesome to be able to load one using `load_dataset` in streaming mode (either from a local directory, or from a dataset on the Hugging Face Hub). Some datasets on the Hub are already in webdataset format.

It terms of implementation, we can have something similar to the Parquet loader.
I also think it's fine to have webdataset as an optional dependency.

Comments: I like the idea of having `webdataset` as an optional dependency to ensure our loader generates web datasets the same way as the main project.

Webdataset is the one of the most popular dataset formats for large scale computer vision tasks. Upvote for this issue. 

Any updates on this?

We haven't had the bandwidth to implement it so far, but if someone wants to give it a shot please don't hesitate ^^

Done in #6391
Set `IterableDataset.map` param `batch_size` typing as optional
State: closed | Author: alvarobartt (MEMBER)

This PR solves #5325 

~Indeed we're using the typing for optional values as `Union[type, None]` as it's similar to how Python 3.10 handles optional values as `type | None`, instead of using `Optional[type]`.~

~Do we want to start using `Union[type, None]` for type-hinting optional values or just keep on using `Optional`?~ -> Keeping `Optional` still for consistency with the rest of the code in `datasets`

Also we now allow `batch_size` to be `None` for `IterableDataset.map` and `IterableDataset.filter`e.g. `MappedExamplesIterable` as `map` is internally instantiating those and propagating the `batch_size` param so if it can be `None` for `map` it should also do so for `MappedExamplesIterable`, as well as for `FilteredExamplesIterable` when calling `IterableDataset.filter`.

## TODOs

- [x] Add integration tests
- [x] Handle scenario where `batched=True` and `batch_size=None` or `batch_size<=0`

Comments: The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/datasets/pr_5336). All of your documentation changes will be reflected on that endpoint.

Hi @mariosasko, @lhoestq I was wondering whether we should include `batched` as a `pytest.mark` param for the functions testing `IterableDataset.map` so as to ensure that the changes done in this PR work fine without breaking anything of the actual functionality.

I've pushed updated tests just for one of the unit testing functions to be run as `pytest tests/test_iterable_dataset.py::test_mapped_examples_iterable -s --durations 0`, but some are still missing `batched` param, it was just to ask you whether we're supposed to do this for the rest of the functions or not, if it's a yes I'll push the commit as it's ready, but didn't want to push extra stuff that may be discarded later!

Thanks :hugs:

Thanks for the feedback @lhoestq, I agree with keeping `Optional` instead of `Union[type, None]` for now 👍🏻
Update tasks.json
State: closed | Author: sayakpaul (MEMBER)

Context:

* https://github.com/huggingface/datasets/issues/5255#issuecomment-1339107195

Cc: @osanseviero 

Comments: _The documentation is not available anymore as the PR was closed or merged._

I think the only place where we need to add it is here https://github.com/huggingface/hub-docs/blob/main/js/src/lib/interfaces/Types.ts

And I think we can remove tasks.json completely from this repo

Isn't tasks.json used anymore in this repo?

> I think the only place where we need to add it is here https://github.com/huggingface/hub-docs/blob/main/js/src/lib/interfaces/Types.ts
> 
> And I think we can remove tasks.json completely from this repo

What about the warning I mentioned in https://github.com/huggingface/datasets/issues/5255#issuecomment-1339013527? Also, the depth estimation entry is already present in https://github.com/huggingface/hub-docs/blob/main/js/src/lib/interfaces/Types.ts. 

The update is based on what I received in the output of the export job (c.f. https://github.com/huggingface/datasets/issues/5255#issuecomment-1339107195). 

Edit: Oh, are you referring to the dataset card of NYU Depth V2?

Yes, my suggestion was for the dataset card: you got the error message because you tried to set `depth-estimation` in `class_ids` instead of `class_categories`.

> What about the warning I mentioned in https://github.com/huggingface/datasets/issues/5255#issuecomment-1339013527? Also, the depth estimation entry is already present in https://github.com/huggingface/hub-docs/blob/main/js/src/lib/interfaces/Types.ts.

if you place it in `task_categories` you should be good :)

yes i would suggest rm'ing tasks.json here for clarity

Closing it. 

It's not clear if we can remove it btw, since old versions of `evaluate` rely on it (see https://github.com/huggingface/evaluate/pull/309)

cc @lvwerra 

Actually it can be removed without incidence in old versions of evaluate since we kept an hardcoded `known_task_ids` that is marked "DEPRECATED"
Clean up docstrings
State: closed | Author: stevhliu (MEMBER)
Labels: name: documentation

As raised by @polinaeterna in #5324, some of the docstrings are a bit of a mess because it has both Markdown and Sphinx syntax. This PR fixes the docstring for `DatasetBuilder`. 

I'll start working on cleaning up the rest of the docstrings and removing the old Sphinx syntax (let me know if you prefer one big PR with all the cleaned changes or multiple smaller ones)! 🧼 

Comments: _The documentation is not available anymore as the PR was closed or merged._

Thanks ! Let us know if we can help :)

Small pref for having multiple PRs

Awesome, thanks! Sorry this one is a little big, I'll open some smaller ones next :)
Clarify imagefolder is for small datasets
State: closed | Author: stevhliu (MEMBER)

Based on feedback from [here](https://github.com/huggingface/datasets/issues/5317#issuecomment-1334108824), this PR adds a note to the `imagefolder` loading and creating docs that `imagefolder` is designed for small scale image datasets.

Comments: _The documentation is not available anymore as the PR was closed or merged._

I think it's also reasonable to add the same note to the AudioFolder decription

Thank you ! I think "regular" is more appropriate than "small". It can easily scale to a few thousands of images - just not millions x)

Replaced "small" with "several thousand" since what is considered "regular" and even "small" can be kind of vague!
Fix docs building for main
State: closed | Author: albertvillanova (MEMBER)

This PR reverts the triggering event for building documentation introduced by:
- #5250

Fix #5326.

Comments: _The documentation is not available anymore as the PR was closed or merged._

EDIT
At least the docs for ~~main~~ PR branch are now built:
- https://github.com/huggingface/datasets/actions/runs/3594847760/jobs/6053620813

Build documentation for main branch was triggered after this PR being merged: https://github.com/huggingface/datasets/actions/runs/3603370082/jobs/6071482470
map(...batch_size=None) for IterableDataset
State: closed | Author: frankier (CONTRIBUTOR)
Labels: name: enhancement

name: good first issue

### Feature request

Dataset.map(...) allows batch_size to be None. It would be nice if IterableDataset did too.

### Motivation

Although it may seem a bit of a spurious request given that `IterableDataset` is meant for larger than memory datasets, but there are a couple of reasons why this might be nice.

One is that load_dataset(...) can return either IterableDataset or Dataset. mypy will then complain if batch_size=None even if we know it is Dataset. Of course we can do:

    assert isinstance(d, datasets.DatasetDict)

But it is a mild inconvenience. What's more annoying is that whenever we use something like e.g. `combine_datasets(...)`, we end up with the union again, and so have to do the assert again.

Another is that we could actually end up with an IterableDataset small enough for memory in normal/correct usage, e.g. by filtering a massive IterableDataset.

For practical usages, an alternative to this would be to convert from an iterable dataset to a map-style dataset, but it is not obvious how to do this.

### Your contribution

Not this time.

Comments: Hi! I agree it makes sense for `IterableDataset.map` to support the `batch_size=None` case. This should be super easy to fix.

@mariosasko as this is something simple maybe I can include it as part of https://github.com/huggingface/datasets/pull/5311? Let me know :+1:

#self-assign

Feel free to close this @lhoestq as part of https://github.com/huggingface/datasets/pull/5336 :hugs:

Thanks again :)

> For practical usages, an alternative to this would be to convert from an iterable dataset to a map-style dataset, but it is not obvious how to do this.

This is interesting as well, if anyone wants to explore
Fix docstrings and types in documentation that appears on the website
State: open | Author: polinaeterna (CONTRIBUTOR)
Labels: name: documentation

While I was working on https://github.com/huggingface/datasets/pull/5313 I've noticed that we have a mess in how we annotate types and format args and return values in the code. And some of it is displayed in the [Reference section](https://huggingface.co/docs/datasets/package_reference/builder_classes) of the documentation on the website.
 
Would be nice someday, maybe before releasing datasets 3.0.0, to unify it......

Comments: I agree we have a mess with docstrings...

Ok, I believe we've cleaned up most of the old syntax we were using for the user-facing docs! There are still a couple of `:obj:`'s and `:class:` floating around in the docstrings we don't expose that I'll track down :)

Hi @polinaeterna @albertvillanova @stevhliu,  I hope you all are doing well.

Is this issue still unresolved as I am interested in it?

It should be mostly fixed for the user-facing APIs, but there may be some Sphinx syntax still lurking around in the non-public APIs. Feel free to open a PR to fix those if you catch any! 🤗 

Thanks for your reply @stevhliu  :)
Sure, I will try to find out the remaining and fix that.
Raise error for `.tar` archives in the same way as for `.tar.gz` and `.tgz` in `_get_extraction_protocol`
State: closed | Author: polinaeterna (CONTRIBUTOR)

Currently `download_and_extract` doesn't throw an error when it is used with files with `.tar` extension in streaming mode because `_get_extraction_protocol` doesn't do it (like it does for  `tar.gz` and `tgz`). `_get_extraction_protocol` returns formatted url as if we support tar protocol but we don't.
That means that in dataset scripts `.tar` files would be attempted to load and fail during examples generation (after `download_and_extract` execution). So this PR raises error for `tar` files too.


Comments: _The documentation is not available anymore as the PR was closed or merged._
Fix loading from HF GCP cache
State: closed | Author: lhoestq (MEMBER)

As reported in https://discuss.huggingface.co/t/error-loading-wikipedia-dataset/26599/4 it's not possible to download a cached version of Wikipedia from the HF GCP cache

I fixed it and added an integration test (runs in 10sec)

Comments: _The documentation is not available anymore as the PR was closed or merged._

> Do you know why this stopped working?

It comes from the changes in https://github.com/huggingface/datasets/pull/5107/files#diff-355ae5c229f95f86895404b72378ecd6e966c41cbeebb674af6fe6e9611bc126
[Extract] Place the lock file next to the destination directory
State: closed | Author: lhoestq (MEMBER)

Previously it was placed next to the archive to extract, but the archive can be in a read-only directory as noticed in https://github.com/huggingface/datasets/issues/5295

Therefore I moved the lock location to be next to the destination directory, which is required to have write permissions

Comments: _The documentation is not available anymore as the PR was closed or merged._
Origin/fix missing features error
State: closed | Author: eunseojo (CONTRIBUTOR)

This fixes the problem of when the dataset_load function reads a function with "features" provided but some read batches don't have columns that later show up. For instance, the provided "features" requires columns A,B,C but only columns B,C show. This fixes this by adding the column A with nulls. 

Comments: _The documentation is not available anymore as the PR was closed or merged._

please review :) @lhoestq @ola13 thankoo

Thanks :) I just updated the test to make sure it works even when there's a column missing, and did a minor change to json.py to add the missing columns for the other kinds of JSON files as well (I moved the code to`self._cast_table`)

Thanks Unso! If @lhoestq is happy then I'm also happy :D

When I noticed the ping, this PR had already been merged...

Luckily, PyArrow's `read_json` behaves the same when `explicit_schema` is given via `ParseOptions`, so I'm okay with this change (our JSON loader doesn't use `read_json` for decoding JSON in some scenarios, so this manual approach is the right one).
Bug in sample_by="paragraph"
State: closed | Author: adampauls (NONE)

### Describe the bug

I think [this line](https://github.com/huggingface/datasets/blob/main/src/datasets/packaged_modules/text/text.py#L96) is wrong and should be `batch = f.read(self.config.chunksize)`. Otherwise it will never terminate because even when `f` is finished reading, `batch` will still be truthy from the last iteration.

### Steps to reproduce the bug

```
> cat test.txt
a b c

d e f
````

```python
>>> import datasets
>>> datasets.load_dataset("text", data_files={"train":"test.txt"}, sample_by="paragraph")
```

This will go on forever.

### Expected behavior

Terminates very quickly. 

### Environment info

`version = "2.6.1"` but I think the bug is still there on main. 

Comments: Thanks for reporting, @adampauls.

We are having a look at it.
Datasets: classification_report() got an unexpected keyword argument 'suffix'
State: closed | Author: JonathanAlis (NONE)

https://github.com/huggingface/datasets/blob/main/metrics/seqeval/seqeval.py


> import datasets
predictions = [['O', 'O', 'B-MISC', 'I-MISC', 'I-MISC', 'I-MISC', 'O'], ['B-PER', 'I-PER', 'O']]
references = [['O', 'O', 'O', 'B-MISC', 'I-MISC', 'I-MISC', 'O'], ['B-PER', 'I-PER', 'O']]
seqeval = datasets.load_metric("seqeval")
results = seqeval.compute(predictions=predictions, references=references)
print(list(results.keys()))
print(results["overall_f1"])
print(results["PER"]["f1"])

It raises the error:

> TypeError: classification_report() got an unexpected keyword argument 'suffix'

For context, versions on my pip list -v

> datasets                 1.12.1  
seqeval                  1.2.2

Comments: This seems similar to https://github.com/huggingface/datasets/issues/2512 Can you try to update seqeval ? 

@JonathanAlis also note that the metrics are deprecated in our `datasets` library.

Please, use the new library 🤗 Evaluate instead: https://huggingface.co/docs/evaluate
Fix description of streaming in the docs
State: closed | Author: polinaeterna (CONTRIBUTOR)

We say that "the data is being downloaded progressively" which is not true, it's just streamed, so I fixed it. Probably I missed some other places where it is written? 

Also changed docstrings for `StreamingDownloadManager`'s `download` and `extract` to reflect the same, as these docstrings are displayed in the documentation cc @lhoestq 

Comments: _The documentation is not available anymore as the PR was closed or merged._
Add `features` param to `IterableDataset.map`
State: closed | Author: alvarobartt (MEMBER)

## Description

As suggested by @lhoestq in #3888, we should be adding the param `features` to `IterableDataset.map` so that the features can be preserved (not turned into `None` as that's the default behavior) whenever the user passes those as param, so as to be consistent with `Dataset.map`, as it provides the `features` param so that those are not inferred by default, but specified by the user, and later validated by `ArrowWriter`.

This is internally handled already by the functions relying on `IterableDataset.map` such as `rename_column`, `rename_columns`, and `remove_columns` as described in #5287.

## Usage Example

```python
from datasets import load_dataset, Features

ds = load_dataset("rotten_tomatoes", split="validation", streaming=True)
print(ds.info.features)
ds = ds.map(
    lambda x: {"target": x["label"]},
    features=Features(
        {"target": ds.info.features["label"], "label": ds.info.features["label"], "text": ds.info.features["text"]}
    ),
)
print(ds.info.features)
```

Comments: _The documentation is not available anymore as the PR was closed or merged._
Support xPath for Windows pathnames
State: closed | Author: albertvillanova (MEMBER)

This PR implements a string representation of `xPath`, which is valid for local paths (also windows) and remote URLs.

Additionally, some `os.path` methods are fixed for remote URLs on Windows machines.

Now, on Windows machines:
```python
In [2]: str(xPath("C:\\dir\\file.txt"))
Out[2]: 'C:\\dir\\file.txt'
In [3]: str(xPath("http://domain.com/file.txt"))
Out[3]: 'http://domain.com/file.txt'
```

Comments: _The documentation is not available anymore as the PR was closed or merged._
Close stream in `ArrowWriter.finalize` before inference error
State: closed | Author: mariosasko (COLLABORATOR)

Ensure the file stream is closed in `ArrowWriter.finalize` before raising the `SchemaInferenceError` to avoid the `PermissionError` on Windows in `incomplete_dir`'s `shutil.rmtree`.

Comments: _The documentation is not available anymore as the PR was closed or merged._
timit_asr doesn't load the test split.
State: closed | Author: seyong92 (NONE)

### Describe the bug

When I use the function ```timit = load_dataset('timit_asr', data_dir=data_dir)```, it only loads train split, not test split.

I tried to change the directory and filename to lower case to upper case for the test split, but it does not work at all.

```python
DatasetDict({
    train: Dataset({
        features: ['file', 'audio', 'text', 'phonetic_detail', 'word_detail', 'dialect_region', 'sentence_type', 'speaker_id', 'id'],
        num_rows: 4620
    })
    test: Dataset({
        features: ['file', 'audio', 'text', 'phonetic_detail', 'word_detail', 'dialect_region', 'sentence_type', 'speaker_id', 'id'],
        num_rows: 0
    })
})
```

The directory structure of both splits are same. (DIALECT_REGION / SPEAKER_CODE / DATA_FILES)

### Steps to reproduce the bug

1. just use ```timit = load_dataset('timit_asr', data_dir=data_dir)```

### Expected behavior

```python
DatasetDict({
    train: Dataset({
        features: ['file', 'audio', 'text', 'phonetic_detail', 'word_detail', 'dialect_region', 'sentence_type', 'speaker_id', 'id'],
        num_rows: 4620
    })
    test: Dataset({
        features: ['file', 'audio', 'text', 'phonetic_detail', 'word_detail', 'dialect_region', 'sentence_type', 'speaker_id', 'id'],
        num_rows: 1680
    })
})
```

### Environment info

- ubuntu 20.04
- python 3.9.13
- datasets 2.7.1

Comments: The [timit_asr.py](https://huggingface.co/datasets/timit_asr/blob/main/timit_asr.py) script iterates over the WAV files per split directory using this:
```python
wav_paths = sorted(Path(data_dir).glob(f"**/{split}/**/*.wav"))
wav_paths = wav_paths if wav_paths else sorted(Path(data_dir).glob(f"**/{split.upper()}/**/*.WAV"))
```

Can you check that there is a directory named "test" somewhere in your timit data directory ?
Improve `use_auth_token` docstring and deprecate `use_auth_token` in `download_and_prepare`
State: closed | Author: mariosasko (COLLABORATOR)

Clarify in the docstrings what happens when `use_auth_token` is `None` and deprecate the `use_auth_token` param in `download_and_prepare`.

Comments: _The documentation is not available anymore as the PR was closed or merged._
Return a split Dataset in load_dataset
State: closed | Author: lhoestq (MEMBER)

...instead of a DatasetDict.

```python
# now supported
ds = load_dataset("squad")
ds[0]  
for example in ds:
    pass

# still works
ds["train"]
ds["validation"]

# new
ds.splits  # Dict[str, Dataset] | None

# soon to be supported (not in this PR)
ds = load_dataset("dataset_with_no_splits")
ds[0]
for example in ds:
    pass
```

I implemented `Dataset.__getitem__` and `IterableDataset.__getitem__` to be able to get a split from a dataset.
The splits are defined by the `ds.info.splits` dictionary.

Therefore a dataset is a table that optionally has some splits defined in the dataset info. And a split dataset is the concatenation of all its splits.

I made as little breaking changes as possible. Notable breaking changes:
- `load_dataset("potato").keys() / .items() / .values() /` don't work anymore, since we don't return a dict
- same for `for split_name in load_dataset("potato")`, since we now iterate on the examples
- ..

TODO:
- [x] Update push_to_hub
- [x] Update save_to_disk/load_from_disk
- [ ] check for other breaking changes
- [ ] fix existing tests
- [ ] add new tests
- [ ] docs

This is related to https://github.com/huggingface/datasets/issues/5189, to extend `load_dataset` to return datasets without splits

Comments: The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/datasets/pr_5301). All of your documentation changes will be reflected on that endpoint.

Just noticed that now we have to deal with indexed & split datasets. The remaining tests are failing because one should be able to get an indexed dataset when accessing the split of a dataset made of indexed splits (right now the index is just trashed)
Use same `num_proc` for dataset download and generation
State: closed | Author: mariosasko (COLLABORATOR)

Use the same `num_proc` value for data download and generation. Additionally, do not set `num_proc` to 16 in `DownloadManager` by default (`num_proc` now has to be specified explicitly).

Comments: _The documentation is not available anymore as the PR was closed or merged._

I noticed this bug the other day and was going to look into it! "Where are these processes coming from?" ;-)
Bug in xopen with Windows pathnames
State: closed | Author: albertvillanova (MEMBER)
Labels: name: bug

Currently, `xopen` function has a bug with local Windows pathnames:

From its implementation:
```python
def xopen(file: str, mode="r", *args, **kwargs):
    file = _as_posix(PurePath(file))
    main_hop, *rest_hops = file.split("::")
    if is_local_path(main_hop):
        return open(file, mode, *args, **kwargs)
```

On a Windows machine, if we pass the argument:
```python
xopen("C:\\Users\\USERNAME\\filename.txt")
```
it returns
```python
open("C:/Users/USERNAME/filename.txt")
```
Bug in xjoin with Windows pathnames
State: closed | Author: albertvillanova (MEMBER)
Labels: name: bug

Currently, `xjoin` function has a bug with local Windows pathnames: instead of returning the OS-dependent join pathname, it always returns it in POSIX format.

```python
from datasets.download.streaming_download_manager import xjoin

path = xjoin("C:\\Users\\USERNAME", "filename.txt")
```

Join path should be:
```python
"C:\\Users\\USERNAME\\filename.txt"
```
However it is:
```python
"C:/Users/USERNAME/filename.txt"
```