helmo/github-issues huggingface dataset:
Protect master branch to force contributions via Pull Requests
State: closed | Author: albertvillanova (MEMBER)
Labels: name: enhancement

In order to have a clearer Git history in the master branch, I propose to protect it so that all contributions must be done through a Pull Request and no direct commits to master are allowed.
- The Pull Request allows to give context, discuss any potential issues and improve the quality of the contribution
- The Pull Request will eventually be squashed and merged into master with a single commit that links to the Pull Request page (with all the context/discussions)

Note that we already implemented a protection in the master branch to avoid *merge* commits and ensure a linear history. This proposal goes one step further by avoiding all kind of direct commits and forcing contributions **only** through Pull Requests.

Please note that we can temporarily deactivate this protection if we need to make a direct commit, e.g. at each new version release.

The only way GitHub allows this kind or protection is by requiring a minimal number (at least one) of approvals of the Pull Request. The inconvenient is that the PR creator cannot approve their own PR: another person must approve it before it can be merged into master. To circumvent this, we could eventually disable this protection in the master branch when an urgent commit is needed (e.g. for a hotfix) and there is no other person available at that time to approve the PR.


Comments: It would be nice to protect the master from direct commits, but still having a way to merge our own PRs when no review is required (for example when updating a dataset_infos.json file, or minor bug fixes - things that happen quite often actually).
Do you know if there's a way ?

you can if you're an admin of the repo

This is done. Now the master branch is protected:
- [x] Require a pull request before merging: all commits must be made to a non-protected branch and submitted via a pull request
  - Required number of approvals before merging: 1 
- [x] Require linear history: prevent merge commits from being pushed
- [x] These requirements are not enforced for administrators
- [x] Additionally, the master branch is also protected against deletion and force pushes

CC: @lhoestq @julien-c @thomwolf
Errors loading dataset using fs = a gcsfs.GCSFileSystem
State: open | Author: dconatha (NONE)
Labels: name: bug

## Describe the bug

Cannot load dataset using a `gcsfs.GCSFileSystem`.  I'm not sure if this should be a bug in `gcsfs` or here...

Basically what seems to be happening is that since datasets saves datasets as folders and folders aren't "real objects" in gcs, gcsfs raises a 404 error.  There are workarounds if you use gcsfs directly to download the file, but as is I can't get `load_from_disk` to work.

## Steps to reproduce the bug
```python
from datasets import load_dataset
# load some dataset
dataset = load_dataset("squad", split="train")

# save it to gcs
import gcsfs
fs = gcsfs.GCSFileSystem(project="my-gs-project")
dataset.save_to_disk("gs://my-bucket/squad", fs=fs)

# try to load it from gcs
from datasets import load_from_disk
dataset2 = load_from_disk("my-bucket/squad", fs=fs)
```


## Expected results
`dataset2` would be a copy of `dataset` but loaded from my bucket.

## Actual results
Long traceback but essentially it's a 404 error from gcsfs saying the object `my-bucket/squad` doesn't exist when this is called:
https://github.com/huggingface/datasets/blob/9c81b7d2e6d9feae69a084a3abda265a4ca07fb5/src/datasets/arrow_dataset.py#L977

This is because there is no actual object called `my-bucket/squad`, there are objects called `my-bucket/squad/dataset.arrow`, etc.

Note that *this* works fine, since it's explicitly saying "download all the objects with this prefix":
```python
            fs.download(src_dataset_path + "/*", dataset_path.as_posix(), recursive=True)
```
For example, I can do a workaround this way:
```python
import tempfile

with tempfile.TemporaryDirectory() as temppath:
    fs.download("gs://my-bucket/squad/*", temppath)
    dataset2 = load_from_disk(temppath)
```

It's unclear to me if it's `gcsfs`'s responsibility to say "hey that's folder not a file, I should try to get objects inside of it not the object itself", or if that's `datasets`'s responsibility... I'm leaning towards the latter since you're never loading a dataset from one file using this function/method, only a dataset folder?

Another minor thing that should maybe should be rolled into this bug...

https://github.com/huggingface/datasets/blob/9c81b7d2e6d9feae69a084a3abda265a4ca07fb5/src/datasets/arrow_dataset.py#L968

These fail if you pass in a `gs://` path, e.g.

```python
dataset2 = load_from_disk("gs://my-bucket/squad", fs=fs)
```
Because at this point, `dataset_info_path` is `gs:/my-bucket/squad/dataset_info.json`, gcsfs throws a:
```
Invalid bucket name: 'gs:'
```
error



## Environment info
<!-- You can run the command `datasets-cli env` and copy-and-paste its output below. -->
- `datasets` version: 1.12.1
- Platform: macOS Big Sur 11.6
- Python version: 3.7.12
- PyArrow version: 5.0.0
Actual "proper" install of ruamel.yaml in the windows CI
State: closed | Author: lhoestq (MEMBER)

It was impossible to update the package directly with `pip`. Indeed it was installed with `distutils` which prevents `pip` or `conda` to uninstall it.

I had to `rm` a directory from the `site-packages` python directory, and then do `pip install ruamel.yaml`

It's not that "proper" but I couldn't find better solutions
Error when loading private dataset with "data_files" arg
State: closed | Author: borisdayma (CONTRIBUTOR)
Labels: name: bug

## Describe the bug
A clear and concise description of what the bug is.

Private datasets with no loading script can't be loaded using `data_files` parameter.

## Steps to reproduce the bug
```python
from datasets import load_dataset
data_files = {"train": "**/train/*/*.jsonl", "valid": "**/valid/*/*.jsonl"}
dataset = load_dataset('dalle-mini/encoded', data_files=data_files, use_auth_token=True, streaming=True)
```

Same error happens in non-streaming mode.

## Expected results
Files should be loaded (whether in streaming or not).

## Actual results
Error:

```
---------------------------------------------------------------------------
FileNotFoundError                         Traceback (most recent call last)
/usr/local/lib/python3.7/dist-packages/datasets/load.py in prepare_module(path, script_version, download_config, download_mode, dataset, force_local_path, dynamic_modules_path, return_resolved_file_path, return_associated_base_path, data_files, **download_kwargs)
    539                 try:
--> 540                     local_path = cached_path(file_path, download_config=download_config)
    541                 except FileNotFoundError:

8 frames
FileNotFoundError: Couldn't find file at https://huggingface.co/datasets/dalle-mini/encoded/resolve/main/encoded.py

During handling of the above exception, another exception occurred:

HTTPError                                 Traceback (most recent call last)
HTTPError: 404 Client Error: Not Found for url: https://huggingface.co/api/datasets/dalle-mini/encoded?full=true

During handling of the above exception, another exception occurred:

FileNotFoundError                         Traceback (most recent call last)
/usr/local/lib/python3.7/dist-packages/datasets/load.py in prepare_module(path, script_version, download_config, download_mode, dataset, force_local_path, dynamic_modules_path, return_resolved_file_path, return_associated_base_path, data_files, **download_kwargs)
    547                     except Exception:
    548                         raise FileNotFoundError(
--> 549                             f"Couldn't find a directory or a {resource_type} named '{path}'. "
    550                             f"It doesn't exist locally at {expected_dir_for_combined_path_abs} or remotely on {hf_api.endpoint}/datasets"
    551                         )

FileNotFoundError: Couldn't find a directory or a dataset named 'dalle-mini/encoded'. It doesn't exist locally at /content/dalle-mini/encoded or remotely on https://huggingface.co/datasets
```

## Environment info
<!-- You can run the command `datasets-cli env` and copy-and-paste its output below. -->
- `datasets` version: 1.12.1
- Platform: Linux-5.4.104+-x86_64-with-Ubuntu-18.04-bionic
- Python version: 3.7.12
- PyArrow version: 3.0.0

@lhoestq 

Comments: We'll do a release tomorrow or on wednesday to make the fix available :)

Thanks for reproting !
Align tqdm control with cache control
State: closed | Author: mariosasko (COLLABORATOR)

Currently, once disabled with `disable_progress_bar`, progress bars cannot be re-enabled again. To overcome this limitation, this PR introduces the `set_progress_bar_enabled` function that accepts a boolean indicating whether to display progress bars. The goal is to provide a similar API to the existing cache control API. Following the Zen of Python (😄), there should be one and preferably only one obvious way to do it, so I'm also deprecating the aforementioned `disable_progress_bar` function. Additionally, I justify the deprecation with the fact that this function has never been in the docs.

Moreover, similar API changes have recently been introduced to [`tfds`](https://github.com/tensorflow/datasets/blob/a1e8b98f45b0214082b546cc967c67c43fffda55/tensorflow_datasets/core/utils/tqdm_utils.py#L98-L112).

Considering the popularity of the [comment](https://github.com/huggingface/datasets/issues/1627#issuecomment-751383559) I made a while ago, this API (`set_progress_bar_enabled` and `is_progress_bar_enabled`) should be mentioned in the docs, but I'm not sure where to put it exactly. Maybe we can replace the `logging_methods` page under `package_reference` with `utility_methods` and then introduce two subsections on that page: `Logging methods` and `tqdm control`.

Additionally, this PR:
* adds the `disable_tqdm` keyword arg of `Dataset._map_single` to the `ignore_kwargs` list to ignore it when computing the fingerprint (forgot to add it in #2696)
* deletes the unused components in `tqdm_utils.py`, which seem to be inherited from `tfds`
* disables the tqdm output in the test suite. As I see it, this output doesn't seem informative, but let me know if this is not a good idea

Comments: Could you add this function to the documentation please ?

You can add it in `main_classes.rst`, and maybe add a `Tip` section in the `map` section in the `process.rst`
Add `remove_columns` to `IterableDataset`
State: closed | Author: changjonathanc (CONTRIBUTOR)

Fixes #2944

WIP
* Not tested yet.
* We might want to allow batched remove for efficiency.

@lhoestq Do you think it should have `batched=` and `batch_size=`?

Comments: Thanks ! That looks all good :)

I don't think that batching would help. Indeed we're dealing with python iterators that yield elements one by one, so batched `map` needs to accumulate a batch, apply the function, and then yield examples from the batch.

Though once we have parallel processing in `map`, we can reconsider it

Also feel free to check the CI failure - apparently the import of `Union` is missing

Thanks for the review and explaining that! 
On top of what you said, I think `remove_columns` is very unlikely to be a bottleneck, so it doesn't matter anyways.

Thank you for reviewing! @mariosasko 

I wonder how the checking would work. Is there any checking present in `IterableDataset ` now? What if `.remove_columns()` is applied after some arbitrary `.map()`?

> I wonder how the checking would work. Is there any checking present in IterableDataset now? What if .remove_columns() is applied after some arbitrary .map()?

That's the challenge here indeed ^^ In this case it's not trivial to know the names of the columns. Feel free to open an issue so we can discuss this
Use standard open-domain validation split in nq_open
State: closed | Author: craffel (CONTRIBUTOR)

The nq_open dataset originally drew the validation set from this file:
https://github.com/google-research-datasets/natural-questions/blob/master/nq_open/NQ-open.efficientqa.dev.1.1.sample.jsonl
However, that's the dev set used specifically and only for the efficientqa competition, and it's not the same dev set as is used in every open-domain question answering paper (including the Lee et al paper that introduced the open-domain variant of NQ, cited at the top of the dataset file). This PR changes nq_open to use the standard validation split and bumps the version to 2.0.0 since this is a breaking change.

Comments: I had to run datasets-cli with --ignore_verifications the first time since it was complaining about a missing file, but now it runs without that flag fine. I moved dummy_data.zip to the new folder, but also had to modify the filename of the test file in the zip (should I not have done that?). Finally, I added the pretty name tag.

Great, thanks for the help.
Properly install ruamel-yaml for windows CI
State: closed | Author: lhoestq (MEMBER)


Comments: @lhoestq I would say this does not "properly" install `ruamel-yaml`, but the contrary, you overwrite the previous version without desinstalling it first.

According to `pip` docs:
>  This can break your system if the existing package is of a different version or was installed with a different package manager!

Note that our case fulfills both conditions:
- the installing version (`0.17.16`) is different from the existing one (`0.15.87`)
- you are installing using `pip` (`setuptools`), whereas the exisitng version was installed using `distutils`

That is why I did not fix the issue this way, made a hotfix pinning `huggingface_hub` (#3025), while looking for a permanent solution for the issue.

Yea I did this because we need the latest version of `huggingface_hub` for #2986 and because I didn't want to ssh to the windows worker x)
Maybe it can be fixed by installing it with conda - let me try

Oh yea it may not work since it was first installed with distutils
Resolve data_files by split name
State: closed | Author: lhoestq (MEMBER)

This issue is about discussing the default behavior when someone loads a dataset that consists in data files. For example:
```python
load_dataset("lhoestq/demo1")
```
should return two splits "train" and "test" since the dataset repostiory is like
```
data/
├── train.csv
└── test.csv
```
Currently it returns only one split "train" which contains the data of both files


I started playing with this idea on this branch btw: `resolve-data_files-by-split-name`
Basically the idea is that if you named you data files after split names then the default pattern is
```python
{
    "train": ["*train*"],
    "test": ["*test*"],
    "validation": ["*dev*", "valid"],
}
```
otherwise it's
```python
{
    "train": ["*"]
}
```

Let me know what you think !
cc @albertvillanova @LysandreJik @vblagoje 

Comments: Awesome @lhoestq I like the proposal and it works great on my JSON community dataset. Here is the [log](https://gist.github.com/vblagoje/714babc325bcbdd5de579fd8e1648892). 

From my discussion with @borisdayma it would be more general the files match if their paths contains the split name - not only if the filename contains the split name. For example for a dataset like this:
```
train/
└── data.csv
test/
└── data.csv
```

But IMO the default should be 
```
data/
├── train.csv
└── test.csv
```
because it allows people to have other directories if they have different subsets of their data (different configurations, not splits)

I just created a PR for this at https://github.com/huggingface/datasets/pull/3221, let me know what you think :)
Windows test suite fails
State: closed | Author: albertvillanova (MEMBER)
Labels: name: bug

## Describe the bug

There is an error during installation of tests dependencies for Windows: https://app.circleci.com/pipelines/github/huggingface/datasets/7981/workflows/9b6a0114-2b8e-4069-94e5-e844dbbdba4e/jobs/49206

```
ERROR: Cannot uninstall 'ruamel-yaml'. It is a distutils installed project and thus we cannot accurately determine which files belong to it which would lead to only a partial uninstall.
```
MeDAL dataset: Add further description and update download URL
State: closed | Author: xhluca (NONE)

Added more details in the following sections:
* Dataset Structure
* Data Instances
* Data Splits
* Source Data
* Annotations
* Discussions of Biases
* LIcensing Information

Comments: @lhoestq I'm a bit confused by the error message. I haven't touched the YAML code at all - do you have any insight on that?

I just added the missing `pretty_name` tag in the YAML - sorry about that ;)

Thanks! Seems like it did the trick since the tests are passing. Let me know if there's anything else I can do in this PR!

It's all good thank you :)

merging !
Add a metric for the MATH dataset (competition_math).
State: closed | Author: hacobe (CONTRIBUTOR)

This metric computes accuracy for the MATH dataset (https://arxiv.org/abs/2103.03874) after canonicalizing the prediction and the reference (e.g., converting "1/2" to "\\\\frac{1}{2}").

Comments: I believe the only failed test related to this PR is tests/test_metric_common.py::LocalMetricTest::test_load_metric_competition_math. It gives the following error:

ImportError: To be able to use this dataset, you need to install the following dependencies['math_equivalence'] using 'pip install git+https://github.com/hendrycks/math.git' for instance'

It fails along with (these fail with ImportError as well):
test_load_metric_bertscore
test_load_metric_bleurt
test_load_metric_comet
test_load_metric_coval

Let me know if there is anything I need to change.

Hi ! The script looks all good thanks :)

To fix the CI you just need to merge `master` into your branch
```
git fetch upstream/master
git merge upstream/master
```

Then you also need to add `math_equivalence` to the list of git packages installed for the tests in `additional-tests-requirements.txt`
https://github.com/huggingface/datasets/blob/ba831e4bcd175ae3d52afbf7d12c4f625bf541b0/additional-tests-requirements.txt#L1-L3

I ran:

git fetch upstream
git merge upstream/master

And I also added  math_equivalence to the list of git packages installed for the tests in additional-tests-requirements.txt

tests/test_metric_common.py fails with the same errors as before. tests/test_dataset_cards.py also fails, but it doesn't look related to this PR (it's an issue datasets/ami/README.md).

@lhoestq Anything else I can do? I re-merged again and am getting the same test failures as described in the previous comment.
Fix filter leaking
State: closed | Author: lhoestq (MEMBER)

If filter is called after using a first transform `shuffle`, `select`, `shard`, `train_test_split`, or `filter`, then it could not work as expected and return examples from before the first transform. This is because the indices mapping was not taken into account when saving the indices to keep when doing the filtering

Affected versions: 1.12.0 and 1.12.1

This should fix #3010
Support multiple zipped CSV data files
State: open | Author: albertvillanova (MEMBER)
Labels: name: enhancement

As requested by @lewtun, support loading multiple zipped CSV data files.

```python
from datasets import load_dataset

url = "https://domain.org/filename.zip"
data_files = {"train": "train_filename.csv", "test": "test_filename.csv"}
dataset = load_dataset("csv", data_dir=url, data_files=data_files)
```


Comments: @lhoestq I would like to draw your attention to the proposed API by @lewtun, using `data_dir` to pass the ZIP URL.

I'm not totally convinced with this... What do you think?

Maybe we could discuss other approaches...

One brainstorming idea: what about using URL chaining with the hop operator in `data_files`?

`data_dir` is currently exclusively used for manually downloaded data.

Maybe we can have an API that only uses data_files as you are suggesting, using URL chaining ?
```python
from datasets import load_dataset

url = "https://domain.org/filename.zip"
data_files = {"train": "zip://train_filename.csv::" + url, "test": "zip://test_filename.csv::" + url}
dataset = load_dataset("csv", data_files=data_files)
```

URL chaining is used by `fsspec` to get access to files in nested filesystems of any kind. Since `fsspec` is being used by `pandas`, `dask` and also extensively by `datasets` I think it would be nice to use it here too

URL chaining sounds super nice to me! And it's also a nice way to leverage the same concepts we currently have in the docs around `fsspec` :)
Improve `get_dataset_infos`?
State: closed | Author: severo (COLLABORATOR)
Labels: name: question

name: dataset-viewer

Using the dedicated function `get_dataset_infos` on a dataset that has no dataset-info.json file returns an empty info:

```
>>> from datasets import get_dataset_infos
>>> get_dataset_infos('wit')
{}
```

While it's totally possible to get it (regenerate it) with:

```
>>> from datasets import load_dataset_builder
>>> builder = load_dataset_builder('wit')
>>> builder.info
DatasetInfo(description='Wikipedia-based Image Text (WIT) Dataset is a large multimodal multilingual dataset. WIT is composed of a curated set\n of 37.6 million entity rich image-text examples with 11.5 million unique images across 108 Wikipedia languages. Its\n size enables WIT to be used as a pretraining dataset for multimodal machine learning models.\n', citation='@article{srinivasan2021wit,\n  title={WIT: Wikipedia-based Image Text Dataset for Multimodal Multilingual Machine Learning},\n  author={Srinivasan, Krishna and Raman, Karthik and Chen, Jiecao and Bendersky, Michael and Najork, Marc},\n  journal={arXiv preprint arXiv:2103.01913},\n  year={2021}\n}\n', homepage='https://github.com/google-research-datasets/wit', license='', features={'b64_bytes': Value(dtype='string', id=None), 'embedding': Sequence(feature=Value(dtype='float64', id=None), length=-1, id=None), 'image_url': Value(dtype='string', id=None), 'metadata_url': Value(dtype='string', id=None), 'original_height': Value(dtype='int32', id=None), 'original_width': Value(dtype='int32', id=None), 'mime_type': Value(dtype='string', id=None), 'caption_attribution_description': Value(dtype='string', id=None), 'wit_features': Sequence(feature={'language': Value(dtype='string', id=None), 'page_url': Value(dtype='string', id=None), 'attribution_passes_lang_id': Value(dtype='string', id=None), 'caption_alt_text_description': Value(dtype='string', id=None), 'caption_reference_description': Value(dtype='string', id=None), 'caption_title_and_reference_description': Value(dtype='string', id=None), 'context_page_description': Value(dtype='string', id=None), 'context_section_description': Value(dtype='string', id=None), 'hierarchical_section_title': Value(dtype='string', id=None), 'is_main_image': Value(dtype='string', id=None), 'page_changed_recently': Value(dtype='string', id=None), 'page_title': Value(dtype='string', id=None), 'section_title': Value(dtype='string', id=None)}, length=-1, id=None)}, post_processed=None, supervised_keys=None, task_templates=None, builder_name='wit', config_name='default', version=0.0.0, splits=None, download_checksums=None, download_size=None, post_processing_size=None, dataset_size=None, size_in_bytes=None)
```

Should we test if info is empty, and in that case regenerate it? Or always generate it?

Comments: To keeps things simple maybe we should use `load_dataset_builder` in `get_dataset_infos`.
`load_dataset_builder` instantiates a builder and runs the _infos() method in order to give you the most up-to-date infos, even if the dataset_infos.json is outdated or missing.