noamaanMulla-03/datasets-issues huggingface dataset:
Allow sequence features for beam + add processed Natural Questions

## Allow Sequence features for Beam Datasets + add Natural Questions

### The issue

The steps of beam datasets processing is the following:
- download the source files and send them in a remote storage (gcs)
- process the files using a beam runner (dataflow)
- save output in remote storage (gcs)
- convert output to arrow in remote storage (gcs)

However it wasn't possible to process `natural_questions` because apache beam's processing outputs parquet files, and it's not yet possible to read parquet files with list features.

### The proposed solution

To allow sequence features for beam I added a workaround that serializes the values using `json.dumps`, so that we end up with strings instead of the original features. Then when the arrow file is created, the serialized objects are transformed back to normal with `json.loads`. Not sure if there's a better way to do it.

### Natural Questions

I was able to process NQ with it, and so I added the json infos file in this PR too.
The processed arrow files are also stored in gcs.
It allows you to load NQ with

```python
from nlp import load_dataset
nq = load_dataset("natural_questions")  # download the 90GB arrow files from gcs and return the dataset
```

### Tests

I added a test case to make sure it works as expected.
Note that the CI will fail because I am updating `natural_questions.py`: it's not synced with the script on S3. It will be synced as soon as this PR is merged.
```
=========================== short test summary info ============================
FAILED tests/test_hf_gcp.py::TestDatasetOnHfGcp::test_script_synced_with_s3_natural_questions/default
```
[Feature request] Add Google Natural Question dataset

Would be great to have https://github.com/google-research-datasets/natural-questions as an alternative to SQuAD.

Labels: name: dataset request

Indeed, I think this one is almost ready cc @lhoestq 

I'm doing the latest adjustments to make the processing of the dataset run on Dataflow

Is there an update to this? It will be very beneficial for the QA community!

Still work in progress :)
The idea is to have the dataset already processed somewhere so that the user only have to download the processed files. I'm also doing it for wikipedia.

Super appreciate your hard work !!
I'll cross my fingers and hope easily loadable wikipedia dataset will come soon. 

Quick update on NQ: due to some limitations I met using apache beam + parquet I was not able to use the dataset in a nested parquet structure in python to convert it to our Apache Arrow format yet.
However we had planned to change this conversion step anyways so we'll make just sure that it enables to process and convert the NQ dataset to arrow.

NQ was added in #427 🎉
[Question] Using/adding a local dataset

Users may want to either create/modify a local copy of a dataset, or use a custom-built dataset with the same `Dataset` API as externally downloaded datasets.

It appears to be possible to point to a local dataset path rather than downloading the external ones, but I'm not exactly sure how to go about doing this.

A notebook/example script demonstrating this would be very helpful.


Hi @zphang,

So you can just give the local path to a dataset script file and it should work.

Here is an example:
- you can download one of the scripts in the `datasets` folder of the present repo (or clone the repo)
- then you can load it with `load_dataset('PATH/TO/YOUR/LOCAL/SCRIPT.py')`

Does it make sense?

Could you give a more concrete example, please? 

I looked up wikitext dataset script from the repo. Should I just overwrite the `data_file` on line 98 to point to the local dataset directory? Would it work for different configurations of wikitext (wikitext2, wikitext103 etc.)?

Or maybe we can use DownloadManager to specify local dataset location? In that case, where do we use DownloadManager instance?

Thanks

Hi @MaveriQ , although what I am doing is to commit a new dataset, but I think looking at imdb script might help.
You may want to use `dl_manager.download_custom`, give it a url(arbitrary string), a custom_download(arbitrary function) and return a path, and finally use _get sample to fetch a sample.

The download manager supports local directories. You can specify a local directory instead of a url and it should work.

Closing this one.
Feel free to re-open if you have other questions :)
Change features vs schema logic

## New logic for `nlp.Features` in datasets

Previously, it was confusing to have `features` and pyarrow's `schema` in `nlp.Dataset`.
However `features` is supposed to be the front-facing object to define the different fields of a dataset, while `schema` is only used to write arrow files.

Changes:
- Remove `schema` field in `nlp.Dataset`
- Make `features` the source of truth to read/write examples
- `features` can no longer be `None` in `nlp.Dataset`
- Update `features` after each dataset transform such as `nlp.Dataset.map`

Todo: change the tests to take these changes into account


I had to make `SplitDict` serializable to be able to copy `DatasetInfo` objects properly.
Serialization was also asked in #389 

One thing I forgot to say here, is that we also want to use the features arguments of `load_dataset` (which goes in the builder’s config) to override the default features of a dataset script.
Cannot unpickle saved .pt dataset with torch.save()/load()

Saving a formatted torch dataset to file using `torch.save()`. Loading the same file fails during unpickling:

```python
>>> import torch
>>> import nlp

>>> squad = nlp.load_dataset("squad.py", split="train")
>>> squad
Dataset(features: {'source_text': Value(dtype='string', id=None), 'target_text': Value(dtype='string', id=None)}, num_rows: 87599)
>>> squad = squad.map(create_features, batched=True)
>>> squad.set_format(type="torch", columns=["source_ids", "target_ids", "attention_mask"])
>>> torch.save(squad, "squad.pt")

>>> squad_pt = torch.load("squad.pt")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/vegarab/.conda/envs/torch/lib/python3.7/site-packages/torch/serialization.py", line 593, in load
    return _legacy_load(opened_file, map_location, pickle_module, **pickle_load_args)
  File "/home/vegarab/.conda/envs/torch/lib/python3.7/site-packages/torch/serialization.py", line 773, in _legacy_load
    result = unpickler.load()
  File "/home/vegarab/.conda/envs/torch/lib/python3.7/site-packages/nlp/splits.py", line 493, in __setitem__
    raise ValueError("Cannot add elem. Use .add() instead.")
ValueError: Cannot add elem. Use .add() instead.
```
where `create_features` is a function that tokenizes the data using `batch_encode_plus` and returns a Dict with `input_ids`, `target_ids` and `attention_mask`. 
```python
def create_features(batch):
    source_text_encoding = tokenizer.batch_encode_plus(
        batch["source_text"],
        max_length=max_source_length,
        pad_to_max_length=True,
        truncation=True)

    target_text_encoding = tokenizer.batch_encode_plus(
        batch["target_text"],
        max_length=max_target_length,
        pad_to_max_length=True,
        truncation=True)

    features = {
        "source_ids": source_text_encoding["input_ids"],
        "target_ids": target_text_encoding["input_ids"],
        "attention_mask": source_text_encoding["attention_mask"]
    }

    return features
```

I found a similar issue in [issue 5267 in the huggingface/transformers repo](https://github.com/huggingface/transformers/issues/5267) which was solved by downgrading to `nlp==0.2.0`. That did not solve this problem, however. 


This seems to be fixed in a non-released version. 

Installing nlp from source
```
git clone https://github.com/huggingface/nlp
cd nlp
pip install .
```
solves the issue.
Keep loading old file even I specify a new file in load_dataset

I used load a file called 'a.csv' by 
```
dataset = load_dataset('csv', data_file='./a.csv')
```
And after a while, I tried to load another csv called 'b.csv'
```
dataset = load_dataset('csv', data_file='./b.csv')
```
However, the new dataset seems to remain the old 'a.csv' and not loading new csv file.

Even worse, after I load a.csv, the load_dataset function keeps loading the 'a.csv' afterward. 

Is this a cache problem?


Labels: name: dataset bug

Same here !

This is the only fix I could come up with without touching the repo's code.
```python
from nlp.builder import FORCE_REDOWNLOAD
dataset = load_dataset('csv', data_file='./a.csv', download_mode=FORCE_REDOWNLOAD, version='0.0.1')
```
You'll have to change the version each time you want to load a different csv file.
If you're willing to add a ```print```, you can go to ```nlp.load``` and add ```print(builder_instance.cache_dir)``` right before the ```return ds``` in the ```load_dataset``` method. It'll print the cache folder, and you'll just have to erase it (and then you won't need the change here above).
Fix csv/json/txt cache dir

The cache dir for csv/json/txt datasets was always the same. This is an issue because it should be different depending on the data files provided by the user.

To fix that, I added a line that use the hash of the data files provided by the user to define the cache dir.

This should fix #444 


I think this is the way to go but I’m afraid this might be a little slow. I was thinking that we could use a high quality very fast non crypto hash like xxhash for these stuff (hashing data files)

Yep good idea, I'll take a look

I tested the hashing speed [here](https://colab.research.google.com/drive/1hlhP84kLIHmOzMRQN1h8x10hKWpXXyud?usp=sharing).
I was able to get 8x speed with `xxhashlib` (42ms vs 345ms for 100MiB of data).
What do you think @thomwolf ?

I added xxhash and some tests
Add package path to sys when downloading package as github archive

This fixes the `coval.py` metric so that imports within the downloaded module work correctly. We can use a similar trick to add the BLEURT metric (@ankparikh)

@thomwolf not sure how you feel about adding to the `PYTHONPATH` from the script. This is the only way I could make it work with my understanding of `importlib` but there might be a more elegant method.

This PR fixes https://github.com/huggingface/nlp/issues/305


Sorry for the long diff, everything after the imports comes from `black` for code quality :/ 

 I think it's fine and I can't think of another way to make the import work anyways.

Maybe we can have the `sys.path` behavior inside `prepare_module` instead ? Currently it seems to come out of nowhere in the code ^^'
We could check if external imports have a `__init__.py` and if it is the case then we can add to directory to the `PYTHONPATH`
Importing downloaded package repository fails

The `get_imports` function in `src/nlp/load.py` has a feature to download a package as a zip archive of the github repository and import functions from the unpacked directory. This is used for example in the `metrics/coval.py` file, and would be useful to add BLEURT (@ankparikh).

Currently however, the code seems to have trouble with imports within the package. For example:
```
import nlp
coval = nlp.load_metric('coval')
```
yields:
```
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/yacine/Code/nlp/src/nlp/load.py", line 432, in load_metric
    metric_cls = import_main_class(module_path, dataset=False)
  File "/home/yacine/Code/nlp/src/nlp/load.py", line 57, in import_main_class
    module = importlib.import_module(module_path)
  File "/home/yacine/anaconda3/lib/python3.7/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 728, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/home/yacine/Code/nlp/src/nlp/metrics/coval/a78807df33ac45edbb71799caf2b3b47e55df4fd690267808fe963a5e8b30952/coval.py", line 21, in <module>
    from .coval_backend.conll import reader  # From: https://github.com/ns-moosavi/coval
  File "/home/yacine/Code/nlp/src/nlp/metrics/coval/a78807df33ac45edbb71799caf2b3b47e55df4fd690267808fe963a5e8b30952/coval_backend/conll/reader.py", line 2, in <module>
    from conll import mention
ModuleNotFoundError: No module named 'conll'
```

Not sure what the fix would be there.

Labels: name: metric bug
Fix KeyboardInterrupt in map and bad indices in select

If you interrupted a map function while it was writing, the cached file was not discarded.
Therefore the next time you called map, it was loading an incomplete arrow file.

We had the same issue with select if there was a bad indice at one point.

To fix that I used temporary files that are renamed once everything is finished.


Thanks @TevenLeScao for finding this issue

Thanks @lhoestq for catching this ❤️
mlsum

Hello, 

The tests for the load_real_data fail, as there is no default language subset to download it looks for a file that does not exist. This bug does not happen when using the load_dataset function, as it asks you to specify a language if you do not, so I submit this PR anyway. The dataset is avalaible on : https://gitlab.lip6.fr/scialom/mlsum_data


Thanks @RachelKer for this PR.

I think the dummy_data structure does not also match. In the `_split_generator` you have something like `os.path.join(downloaded_files["validation"], lang+'_val.jsonl')` but in you dummy_data you have `os.path.join(downloaded_files["validation"], lang+"_val.zip",  lang+'_val.jsonl')`. I think ` jsonl` files should be directly  in the `dummy_data` folder without the sub-folder 

@lhoestq 

Hi @RachelKer :)
Thanks for adding MLSUM !

To fix the CI I think you just have to rebase from master

Great, I think it is working now. Thanks :)

It looks like your PR does tons of changes in other datasets. 
Maybe this is because of the merge from master ?

Hmm, I see, sorry I messed up somewhere. Maybe it's easier if we close the pull request and I do another one ?

Yea if it's easier for you feel free to re-open a PR
Add bleurt

This PR adds the BLEURT metric to the library.

The BLEURT `Metric` downloads a TF checkpoint corresponding to its `config_name` at creation (in the `_info` function). Default is set to `bleurt-base-128`.

Note that the default in the original package is `bleurt-tiny-128`, but they throw a warning and recommend using `bleurt-base-128` instead. I think it's safer to have our users have a functioning metric when they call the default behavior, we'll address discrepancies in the issues/discussions if it comes up.

In addition to the BLEURT file, `load.py` was changed so we can ask users to pip install the required packages from git when they have a `setup.py` but are not on PyPL

cc @ankparikh @tsellam


Sorry one nit: Could we use named arguments for the call to BLEURT?

i.e. 
        scores = self.scorer.score(references=references, candidates=predictions)

(i.e. so it is less bug prone)


Following up on Ankur's comment---we are going to drop support for
positional (not named) arguments in the future releases because it seems to
cause bugs and confusion. I hope it doesn't create too much of a mess.

Le jeu. 30 juil. 2020 à 10:44, ankparikh <notifications@github.com> a
écrit :

> Sorry one nit: Could we use named arguments for the call to BLEURT?
>
> i.e.
> scores = self.scorer.score(references=references, candidates=predictions)
>
> (i.e. so it is less bug prone)
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/huggingface/nlp/pull/455#issuecomment-666414514>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ABTMRNGAN2PMECS5K4DIHJDR6GBMLANCNFSM4PL323FA>
> .
>


> Following up on Ankur's comment---we are going to drop support for positional (not named) arguments in the future releases because it seems to cause bugs and confusion. I hope it doesn't create too much of a mess. Le jeu. 30 juil. 2020 à 10:44, ankparikh <notifications@github.com> a écrit :
> […](#)
> Sorry one nit: Could we use named arguments for the call to BLEURT? i.e. scores = self.scorer.score(references=references, candidates=predictions) (i.e. so it is less bug prone) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <[#455 (comment)](https://github.com/huggingface/nlp/pull/455#issuecomment-666414514)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABTMRNGAN2PMECS5K4DIHJDR6GBMLANCNFSM4PL323FA> .

Changed @ankparikh @tsellam, thanks for taking a look!

We should avoid positional arguments in metrics on our side as well. It's a dangerous source of errors indeed.
Keep features after transform

When applying a transform like `map`, some features were lost (and inferred features were used).
It was the case for ClassLabel, Translation, etc.

To fix that, I did some modifications in the `ArrowWriter`:

- added the `update_features` parameter. When it's `True`, then the features specified by the user (if any) can be updated with inferred features if their type don't match. `map` transform sets `update_features=True` when writing to cache file or buffer. Features won't change by default in `map`.

- added the `with_metadata` parameter. If `True`, the `features` (after update) will be written inside the metadata of the schema in this format:
```
{
    "huggingface": {"features" : <serialized Features exactly like dataset_info.json>}
} 
```
Then, once a dataset is instantiated without info/features, these metadata are used to set the features of the dataset.


One note on features inference:

if an arrow type is `struct of items` where each item is a `list`, then we return a `dict` in which each item is a `Sequence`.
It means that we don't use the Sequence <-> dict swap when we infer features.

It's fine because the swap is generally used in dataset scripts, in which features are defined (inferred features are discarded)

If it's fine for you @thomwolf we can merge this one :) 

Yes this is fine I think!
Fix handling of config files while loading datasets from multiple processes

When loading shards on several processes, each process upon loading the dataset will overwrite dataset_infos.json in <package path>/datasets/<dataset name>/<hash>/dataset_infos.json. It does so every time, even when the target file already exists and is identical. Because multiple processes rewrite the same file in parallel, it creates a race condition when a process tries to load the file, often resulting in a JSON decoding exception because the file is only partially written.

This pull requests partially address this by comparing if the files are already identical before copying over the downloaded copy to the cached destination. There's still a race condition, but now it's less likely to occur if some basic precautions are taken by the library user, e.g., download all datasets to cache before spawning multiple processes.


Ok for this but I think we may want to use the general `filelock` method we are using at other places in the library instead of filecmp (in particular `filelock` take care of being an atomic operation which is safer for concurrent processes)

Ok I see.
Why not use filelock in this case then ?

I think we should 🙂

Thanks for approving my patch.

I agree that if copying is needed then some locking mechanism should be put in place. But, I don't think a file should be needlessly copied without a check. So I guess the flow should be, lock => copy if needed => unlock, and add locks wherever else that file is being accessed.

I'll also add that my personal experience with filelock on a different project hasn't been that great, and on some occasions a process somehow got through the lock -- I've never gotten to the bottom of that but it tainted my view of that module. Perhaps it's been fixed (or I just miss used it), but thought you should know to take steps to test it.
Add pyarrow warning colab

When a user installs `nlp` on google colab, then google colab doesn't update pyarrow, and the runtime needs to be restarted to use the updated version of pyarrow.

This is an issue because `nlp` requires the updated version to work correctly.

In this PR I added en error that is shown to the user in google colab if the user tries to `import nlp` without having restarted the runtime. The error tells the user to restart the runtime.
add DatasetDict

## Add DatasetDict

### Overview

When you call `load_dataset` it can return a dictionary of datasets if there are several splits (train/test for example).
If you wanted to apply dataset transforms you had to iterate over each split and apply the transform.

Instead of returning a dict, it now returns a `nlp.DatasetDict` object which inherits from dict and contains the same data as before, except that now users can call dataset transforms directly from the output, and they'll be applied on each split.

Before:
```python
from nlp import load_dataset

squad = load_dataset("squad")
print(squad.keys())
# dict_keys(['train', 'validation'])
squad = {
    split_name: dataset.map(my_func) for split_name, dataset in squad.items()
}
print(squad.keys())
# dict_keys(['train', 'validation'])
```

Now:
```python
from nlp import load_dataset

squad = load_dataset("squad")
print(squad.keys())
# dict_keys(['train', 'validation'])
squad = squad.map(my_func)
print(squad.keys())
# dict_keys(['train', 'validation'])
```

### Dataset transforms

`nlp.DatasetDict` implements the following dataset transforms:
- map
- filter
- sort
- shuffle

### Arguments

The arguments of the methods are the same except for split-specific arguments like `cache_file_name`.
For such arguments, the expected input is a dictionary `{split_name: argument_value}`
It concerns:
- `cache_file_name` in map, filter, sort, shuffle
- `seed` and `generator` in shuffle


I did the changes in the docstrings and I added a type check in each `DatasetDict` method to make sure all values are of type `Dataset`

Awesome, do you mind adding these in the doc as well?

I added it to the docs (processing + main classes)

I'm trying to follow along with the following about datasets from the docs:

https://huggingface.co/nlp/loading_datasets.html
https://huggingface.co/nlp/processing.html

However the train_test_split method no longer works as it is expecting a dataset, rather than a datsetdict. How would I got about splitting a CSV into a train and test set? 

I'm trying to utilize the Trainer() class, but am having trouble converting my data from a csv into dataset objects to pass in.
Export TFRecord to GCP bucket

Previously, I was writing TFRecords manually to GCP bucket with : `with tf.io.TFRecordWriter('gs://my_bucket/x.tfrecord')`

Since `0.4.0` is out with the `export()` function, I tried it. But it seems TFRecords cannot be directly written to GCP bucket.

`dataset.export('local.tfrecord')` works fine,  
but `dataset.export('gs://my_bucket/x.tfrecord')` does not work. 

There is no error message, I just can't find the file on my bucket...

---

Looking at the code, `nlp` is using `tf.data.experimental.TFRecordWriter`, while I was using `tf.io.TFRecordWriter`.  

**What's the difference between those 2 ? How can I write TFRecords files directly to GCP bucket ?**

@jarednielsen @lhoestq 


Nevermind, I restarted my python session and it worked fine...

---

I had an authentification error, and I authenticated from another terminal. After that, no more error but it was not working. Restarting the sessions makes it work :)