NyssaW/github-issues huggingface dataset:
🐛 Colab : type object 'pyarrow.lib.RecordBatch' has no attribute 'from_struct_array'

I'm trying to load CNN/DM dataset on Colab.

[Colab notebook](https://colab.research.google.com/drive/11Mf7iNhIyt6GpgA1dBEtg3cyMHmMhtZS?usp=sharing)

But I meet this error :

> AttributeError: type object 'pyarrow.lib.RecordBatch' has no attribute 'from_struct_array'


Comments:
It's strange, after installing `nlp` on Colab, the `pyarrow` version seems fine from `pip` but not from python :

```python
import pyarrow

!pip show pyarrow
print("version = {}".format(pyarrow.__version__))
```

> Name: pyarrow
Version: 0.17.0
Summary: Python library for Apache Arrow
Home-page: https://arrow.apache.org/
Author: None
Author-email: None
License: Apache License, Version 2.0
Location: /usr/local/lib/python3.6/dist-packages
Requires: numpy
Required-by: nlp, feather-format
> 
> version = 0.14.1

Ok I just had to restart the runtime after installing `nlp`. After restarting, the version of `pyarrow` is fine.
❓ How to apply a map to all subsets ?

I'm working with CNN/DM dataset, where I have 3 subsets : `train`, `test`, `validation`.

Should I apply my map function on the subsets one by one ?

```python
import nlp

cnn_dm = nlp.load_dataset('cnn_dailymail')
for corpus in ['train', 'test', 'validation']:
         cnn_dm[corpus] = cnn_dm[corpus].map(my_func)
```

Or is there a better way to do this ?

Comments:
That's the way!
❓ How to remove specific rows of a dataset ?

I saw on the [example notebook](https://colab.research.google.com/github/huggingface/nlp/blob/master/notebooks/Overview.ipynb#scrollTo=efFhDWhlvSVC) how to remove a specific column :

```python
dataset.drop('id')
```

But I didn't find how to remove a specific row. 

**For example, how can I remove all sample with `id` < 10 ?**

Comments:
Hi, you can't do that at the moment.

Can you do it by now? Coz it would be awfully helpful!

you can convert dataset object to pandas and remove a feature and convert back to dataset .

That's what I ended up doing too. but it feels like a workaround to a feature that should be added to the datasets class.
🐛 Trying to use ROUGE metric : pyarrow.lib.ArrowInvalid: Column 1 named references expected length 534 but got length 323

I'm trying to use rouge metric.

I have to files : `test.pred.tokenized` and `test.gold.tokenized` with each line containing a sentence.  
I tried :

```python
import nlp

rouge = nlp.load_metric('rouge')
with open("test.pred.tokenized") as p, open("test.gold.tokenized") as g:
    for lp, lg in zip(p, g):
            rouge.add(lp, lg)
```

But I meet following error :

> pyarrow.lib.ArrowInvalid: Column 1 named references expected length 534 but got length 323

---

Full stack-trace :

```
Traceback (most recent call last):
  File "<stdin>", line 3, in <module>
  File "/home/me/.venv/transformers/lib/python3.6/site-packages/nlp/metric.py", line 224, in add
    self.writer.write_batch(batch)
  File "/home/me/.venv/transformers/lib/python3.6/site-packages/nlp/arrow_writer.py", line 148, in write_batch
    pa_table: pa.Table = pa.Table.from_pydict(batch_examples, schema=self._schema)
  File "pyarrow/table.pxi", line 1550, in pyarrow.lib.Table.from_pydict
  File "pyarrow/table.pxi", line 1503, in pyarrow.lib.Table.from_arrays
  File "pyarrow/public-api.pxi", line 390, in pyarrow.lib.pyarrow_wrap_table
  File "pyarrow/error.pxi", line 85, in pyarrow.lib.check_status
pyarrow.lib.ArrowInvalid: Column 1 named references expected length 534 but got length 323
```

(`nlp` installed from source)

Comments:
Can you share your data files or a minimally reproducible example?

Sure, [here is a Colab notebook](https://colab.research.google.com/drive/1uiS89fnHMG7HV_cYxp3r-_LqJQvNNKs9?usp=sharing) reproducing the error.

> ArrowInvalid: Column 1 named references expected length 36 but got length 56

This is because `add` takes as input a batch of elements and you provided only one. I think we should have `add` for one prediction/reference and `add_batch` for a batch of predictions/references. This would make it more coherent with the way we use Arrow.

Let me do this change

Thanks for noticing though. I was mainly used to do `.compute` directly ^^

Thanks @lhoestq it works :)
AttributeError: 'dict' object has no attribute 'info'

I'm trying to access the information of CNN/DM dataset :

```python
cnn_dm = nlp.load_dataset('cnn_dailymail')
print(cnn_dm.info)
```

returns :

> AttributeError: 'dict' object has no attribute 'info'

Comments:
I could access the info by first accessing the different splits :

```python
import nlp

cnn_dm = nlp.load_dataset('cnn_dailymail')
print(cnn_dm['train'].info)
```

Information seems to be duplicated between the subsets :

```python
print(cnn_dm["train"].info == cnn_dm["test"].info == cnn_dm["validation"].info)
# True
```

Is it expected ?

Good point @Colanim ! What happens under the hood when running:

```python
import nlp

cnn_dm = nlp.load_dataset('cnn_dailymail')
```

is that for every split in `cnn_dailymail`, a different dataset object (which all holds the same info) is created. This has the advantages that the datasets are easily separable in a training setup. 
Also note that you can load e.g. only the `train` split of the dataset via:

```python
cnn_dm_train = nlp.load_dataset('cnn_dailymail', split="train")
print(cnn_dm_train.info)
```

I think we should make the `info` object slightly different when creating the dataset for each split - at the moment it contains for example the variable `splits` which should maybe be renamed to `split` and contain only one `SplitInfo` object ...
Couldn't reach CNN/DM dataset

I can't get CNN / DailyMail dataset.

```python
import nlp

assert "cnn_dailymail" in [dataset.id for dataset in nlp.list_datasets()]
cnn_dm = nlp.load_dataset('cnn_dailymail')
```

[Colab notebook](https://colab.research.google.com/drive/1zQ3bYAVzm1h0mw0yWPqKAg_4EUlSx5Ex?usp=sharing)

gives following error :

```
ConnectionError: Couldn't reach https://s3.amazonaws.com/datasets.huggingface.co/nlp/cnn_dailymail/cnn_dailymail.py
```

Comments:
Installing from source (instead of Pypi package) solved the problem.
[Manual downloads] add logic proposal for manual downloads and add wikihow

Wikihow is an example that needs to manually download two files as stated in: https://github.com/mahnazkoupaee/WikiHow-Dataset. 

The user can then store these files under a hard-coded name: `wikihowAll.csv` and `wikihowSep.csv` in this case in a directory of his choice, e.g. `~/wikihow/manual_dir`.

The dataset can then be loaded via:

```python
import nlp
nlp.load_dataset("wikihow", data_dir="~/wikihow/manual_dir")
```

I added/changed so that there are explicit error messages when using manually downloaded files.



Comments:
> Wikihow is an example that needs to manually download two files as stated in: https://github.com/mahnazkoupaee/WikiHow-Dataset.
> 
> The user can then store these files under a hard-coded name: `wikihowAll.csv` and `wikihowSep.csv` in this case in a directory of his choice, e.g. `~/wikihow/manual_dir`.
> 
> The dataset can then be loaded via:
> 
> ```python
> import nlp
> nlp.load_dataset("wikihow", data_dir="~/wikihow/manual_dir")
> ```
> 
> I added/changed so that there are explicit error messages when using manually downloaded files.

wouldn't be nicer if we can have `manual_dir/wikihow`? 

> > Wikihow is an example that needs to manually download two files as stated in: https://github.com/mahnazkoupaee/WikiHow-Dataset.
> > The user can then store these files under a hard-coded name: `wikihowAll.csv` and `wikihowSep.csv` in this case in a directory of his choice, e.g. `~/wikihow/manual_dir`.
> > The dataset can then be loaded via:
> > ```python
> > import nlp
> > nlp.load_dataset("wikihow", data_dir="~/wikihow/manual_dir")
> > ```
> > 
> > 
> > I added/changed so that there are explicit error messages when using manually downloaded files.
> 
> wouldn't be nicer if we can have `manual_dir/wikihow`?

Sure, I mean the user can decide whatever he likes best :-) The path one puts in `data_dir` will be used as the path to the manual dir. `nlp.load_dataset("wikihow", data_dir="~/manual_dir/wikihow")` would work as well as any other path ;-)  

Perfect! You can merge!
Run save infos

I replaced the old checksum file with the new `dataset_infos.json` by running the script on almost all the datasets we have. The only one that is still running on my side is the cornell dialog

Comments:
Haha that cornell dialogue dataset - that ran for 3h on my computer as well. The `generate_examples` method in this script is one of the most inefficient code samples I've ever seen :D 

Indeed it's been 3 hours already
```73111 examples [3:07:48,  2.40 examples/s]```
Add per type scores in seqeval metric

This PR add a bit more detail in the seqeval metric. Now the usage and output are:

```python
import nlp
met = nlp.load_metric('metrics/seqeval')
references = [['O', 'O', 'O', 'B-MISC', 'I-MISC', 'I-MISC', 'O'], ['B-PER', 'I-PER', 'O']]
predictions =  [['O', 'O', 'B-MISC', 'I-MISC', 'I-MISC', 'I-MISC', 'O'], ['B-PER', 'I-PER', 'O']]
met.compute(predictions, references)

#Output: {'PER': {'precision': 1.0, 'recall': 1.0, 'f1': 1.0, 'number': 1}, 'MISC': {'precision': 0.0, 'recall': 0.0, 'f1': 0, 'number': 1}, 'overall_precision': 0.5, 'overall_recall': 0.5, 'overall_f1': 0.5, 'overall_accuracy': 0.8}
```

It is also possible to compute scores for non IOB notations, POS tagging for example hasn't this kind of notation. Add `suffix` parameter:

```python
import nlp
met = nlp.load_metric('metrics/seqeval')
references = [['O', 'O', 'O', 'MISC', 'MISC', 'MISC', 'O'], ['PER', 'PER', 'O']]
predictions =  [['O', 'O', 'MISC', 'MISC', 'MISC', 'MISC', 'O'], ['PER', 'PER', 'O']]
met.compute(predictions, references, metrics_kwargs={"suffix": True})

#Output: {'PER': {'precision': 1.0, 'recall': 1.0, 'f1': 1.0, 'number': 1}, 'MISC': {'precision': 0.0, 'recall': 0.0, 'f1': 0, 'number': 1}, 'overall_precision': 0.5, 'overall_recall': 0.5, 'overall_f1': 0.5, 'overall_accuracy': 0.9}
```

Comments:
LGTM :-) Some small suggestions to shorten the code a bit :-) 

Can you put the kwargs as normal kwargs instead of a dict? (And add them to the kwargs description As well)

@thom Is-it what you meant?

Yes and there is a dynamically generated doc string in the metric script KWARGS DESCRIPTION
Webis tl-dr

Add the Webid TL:DR dataset.

Comments:
Should that rather be in an organization scope, @thomwolf @patrickvonplaten ?

> Should that rather be in an organization scope, @thomwolf @patrickvonplaten ?

I'm a bit indifferent - both would be fine for me!

@jplu - if creating the dummy_data is too tedious, I can do it as well :-) 

There is dummy_data here, no ?

Yeah I think naming it webis/tl_dr would be best @jplu if that works for you

No problem at all!! On it^^

> There is dummy_data here, no ?

Some paths were wrong - the structure is really confusing and the error messages don't really help either - I have to think about how to make this easier to understand!

Hope it was ok that I fiddled with your PR !

> Some paths were wrong - the structure is really confusing and the error message don't really help either - I have to think about how to make this easier to understand!

Oh ok! I haven't noticed that sorry :(

> Hope it was ok that I fiddled with your PR !

Of course it was ok :)

@julien-c Looks like what you have in mind?

```python
import nlp
nlp.load_dataset("datasets/webis", "tl_dr")

#Output: Downloading and preparing dataset webis/tl_dr (download: Unknown size, generated: Unknown size, total: Unknown size) to /home/jplu/.cache/huggingface/datasets/webis/tl_dr/1.0.0...
```

Merging this for now. Maybe we can see whether to rename it in a different PR @julien-c ? 


Hi, 
Author here of the webis-tldr corpus. Any plans on integrating this dataset into the hub? I remember we could access it in the previous versions of the library. If there is a particular issue that I can help with, do let me know.

Thanks!

Hi @shahbazsyed, this dataset _is_ inside the hub but it's namespaced by the organization name `webis`.

You can load it following the steps described in https://huggingface.co/datasets/webis/tl_dr

Here's a Colab showcasing that it works: https://colab.research.google.com/drive/11IrzRVpnMLJZ8_UFFHLR8FhiajjAHRUU?usp=sharing

The reason the code is in S3 and not in this repo is that the dataset is namespaced under the `webis` organization. We don't have a lot of namespaced datasets yet but this should become the main way we add more datasets in the future.
Let us know if that's an issue for you. Thank you!
Replace checksums files by Dataset infos json

### Better verifications when loading a dataset

I replaced the `urls_checksums` directory that used to contain `checksums.txt` and `cached_sizes.txt`, by a single file `dataset_infos.json`. It's just a dict `config_name` -> `DatasetInfo`.

It simplifies and improves how verifications of checksums and splits sizes are done, as they're all stored in `DatasetInfo` (one per config). Also, having already access to `DatasetInfo` enables to check disk space before running `download_and_prepare` for a given config.

The dataset infos json file is user readable, you can take a look at the squad one that I generated in this PR.

### Renaming

According to these changes, I did some renaming:
`save_checksums` -> `save_infos`
`ignore_checksums` -> `ignore_verifications`

for example, when you are creating a dataset you have to run
```nlp-cli test path/to/my/dataset --save_infos --all_configs```
instead of
```nlp-cli test path/to/my/dataset --save_checksums --all_configs```

### And now, the fun part

We'll have to rerun the `nlp-cli test ... --save_infos --all_configs` for all the datasets

-----------------

feedback appreciated !

Comments:
Great! LGTM :-) 

> Ok, really clean!
> I like the logic (not a huge fan of using `_asdict_inner` but it makes sense).
> I think it's a nice improvement!
> 
> How should we update the files in the repo? Run a big job on a server or on somebody's computer who has most of the datasets already downloaded?

Maybe we can split the updates among us...IMO most datasets run very quickly. 
I think I've downloaded 50 datasets and 80% are loaded in <5min,  15% in <1h and then `wmt` which is still downloading (since 12h). 
I deleted my cache because the `wmt` downloads require quite a lot of space, so I only have parts of the `wmt` datasets on my computer. 

@mariamabarham I guess you have downloaded most of the datasets no?
Add list and inspect methods - cleanup hf_api

Add a bunch of methods to easily list and inspect the processing scripts up-loaded on S3:
```python
nlp.list_datasets()
nlp.list_metrics()
# Copy and prepare the scripts at `local_path` for easy inspection/modification.
nlp.inspect_dataset(path, local_path) 
# Copy and prepare the scripts at `local_path` for easy inspection/modification.
nlp.inspect_metric(path, local_path) 
```

Also clean up the `HfAPI` to use `dataclasses` for better user-experience

Comments:
[Datasets] add ted_hrlr

@thomwolf - After looking at `xnli` I think it's better to leave the translation features and add a `translation` key to make them work in our framework. 

The result looks like this:
![Screenshot from 2020-05-12 18-34-43](https://user-images.githubusercontent.com/23423619/81721933-ee1faf00-9480-11ea-9e95-d6557cbd0ce0.png)

you can see that each split has a `translation` key which value is the nlp.features.Translation object. 

That's a simple change. If it's ok for you, I will add dummy data for the other configs and treat the other translation scripts in the same way.

Comments:
Add nbytes + nexamples check

### Save size and number of examples
Now when you do `save_checksums`, it also create `cached_sizes.txt` right next to the checksum file.
This new file stores the bytes sizes and the number of examples of each split that has been prepared and stored in the cache. Example:

```
# Cached sizes: <full_config_name> <num_bytes> <num_examples>
hansards/house/1.0.0/test 22906629 122290
hansards/house/1.0.0/train 191459584 947969
hansards/senate/1.0.0/test 5711686 25553
hansards/senate/1.0.0/train 40324278 182135
```

### Check processing output 

If there is a `caches_sizes.txt`, then each time we run `download_and_prepare` it will make sure that the sizes match. You can set `ignore_checksums=True` if you don't want that to happen.

### Fill Dataset Info

All the split infos and the checksums are now stored correctly in DatasetInfo after `download_and_prepare`

### Check space on disk before running `download_and_prepare`

Check if the space is lower than the sum of the sizes of the files in `checksums.txt` and `cached_files.txt`. This is not ideal though as it considers the files for all configs.

TODO:
A better way to do it would be to have save the `DatasetInfo` instead of the `checksums.txt` and `cached_sizes.txt`, in order to have one file per dataset config (and therefore consider only the sizes of the files for one config and not all of them). It can also be the occasion to factorize all the `download_and_prepare` verifications. Maybe next PR ?

Comments:
Looks good to me! Should we hard code those numbers in the config classes and make sure that when loading a dataset that the numbers match?
WIP adding metrics

Adding the following metrics as identified by @mariamabarham:

1. BLEU:  BiLingual Evaluation Understudy: https://github.com/tensorflow/nmt/blob/master/nmt/scripts/bleu.py,  https://github.com/chakki-works/sumeval/blob/master/sumeval/metrics/bleu.py (multilingual)
2. GLEU:  Google-BLEU:  https://github.com/cnap/gec-ranking/blob/master/scripts/compute_gleu
3. Sacrebleu: https://pypi.org/project/sacrebleu/1.4.8/ (pypi package), https://github.com/mjpost/sacrebleu (github implementation)
4. ROUGE: Recall-Oriented Understudy for Gisting Evaluation: https://github.com/google-research/google-research/tree/master/rouge, https://github.com/chakki-works/sumeval/blob/master/sumeval/metrics/rouge.py (multilingual)
5. Seqeval: https://github.com/chakki-works/seqeval (github implementation), https://pypi.org/project/seqeval/0.0.12/ (pypi package)
6. Coval:  coreference evaluation package for the CoNLL and ARRAU datasets https://github.com/ns-moosavi/coval
7. SQuAD v1 evaluation script
8. SQuAD V2 evaluation script: https://worksheets.codalab.org/rest/bundles/0x6b567e1cf2e041ec80d7098f031c5c9e/contents/blob/
9. GLUE
10. XNLI


Not now:
1. Perplexity: https://github.com/allenai/allennlp/blob/master/allennlp/training/metrics/perplexity.py
2. Spearman: https://github.com/allenai/allennlp/blob/master/allennlp/training/metrics/spearman_correlation.py
3. F1_measure: https://github.com/allenai/allennlp/blob/master/allennlp/training/metrics/f1_measure.py
4. Pearson_corelation: https://github.com/allenai/allennlp/blob/master/allennlp/training/metrics/pearson_correlation.py
5. AUC: https://github.com/allenai/allennlp/blob/master/allennlp/training/metrics/auc.py 
6. Entropy: https://github.com/allenai/allennlp/blob/master/allennlp/training/metrics/entropy.py

Comments:
It's all about my metric stuff so I'll probably merge it unless you want to have a look.

Took the occasion to remove the old doc and requirements.txt
fix overflow check

I did some tests and unfortunately the test
```
pa_array.nbytes > MAX_BATCH_BYTES
```
doesn't work. Indeed for a StructArray, `nbytes` can be less 2GB even if there is an overflow (it loops...).

I don't think we can do a proper overflow test for the limit of 2GB...

For now I replaced it with a sanity check on the first element.

Comments:
JSON script

Add a JSONS script to read JSON datasets from files.

Comments:
The tests for the Wikipedia dataset do not pass anymore with the error:
```
To be able to use this dataset, you need to install the following dependencies ['mwparserfromhell'] using 'pip install mwparserfromhell' for instance'
```

This was an issue on master. You can just rebase from master.

Perfect! Indeed, it worked^^ Thanks @lhoestq 

Currently the dummy_data tests are always green because in a PR the dataset is not yet synchronized with aws. This PR fixes this: https://github.com/huggingface/nlp/pull/140 . 

Could you test `json` locally or wait until the PR: https://github.com/huggingface/nlp/pull/140 is merged ? :-) 

Ok, I will wait #140 to be merged and then rebase :)
[README dummy data tests] README to better understand how the dummy data structure works

In this PR a README.md is added to tests to shine more light on how the dummy data structure works. I try to explain the different possible cases. IMO the best way to understand the logic is to checkout the dummy data structure of the different datasets I mention in the README.md since those are the "edge cases". 

@mariamabarham @thomwolf @lhoestq @jplu - I'd be happy to checkout the dummy data structure and get some feedback on possible improvements.

Comments:
Fix arrow writer for big datasets using writer_batch_size

This PR fixes Yacine's bug.
According to [this](https://github.com/apache/arrow/blob/master/docs/source/cpp/arrays.rst#size-limitations-and-recommendations), it is not recommended to have pyarrow arrays bigger than 2Go.

Therefore I set a default batch size of 100 000 examples per batch. In general it shouldn't exceed 2Go. If it does, I reduce the batch_size on the fly, and I notify the user with a warning.

Comments:
After a quick chat with Yacine : the 2Go test may not be sufficient actually, as I'm looking at the size of the array and not the size of the current_rows. If the test doesn't do the job I think I'll remove it and lower the batch size a bit to be sure that it never exceeds 2Go. I'll do more tests later
[Tests] Test files locally

This PR adds a `aws` and a `local` decorator to the tests so that tests now run on the local datasets. 

By default, the `aws` is deactivated and `local` is activated and `slow` is deactivated, so that only 1 test per dataset runs on circle ci. 

**When local is activated all folders in `./datasets` are tested.**

**Important** When adding a dataset, we should no longer upload it to AWS. The steps are:
1. Open a PR
2. Add a dataset as described in `datasets/README.md`
3. If all tests pass, push to master

Currently we have 49 functional datasets in our code base. 

We have 6 datasets "under-construction" that don't pass the tests - so I put them in a folder "datasets_under_construction" - it would be nice to open a PR to fix them and put them in the `datasets` folder.

**Important** when running tests locally, the datasets are cached so to rerun them delete your local cache via:
`rm -r ~/.cache/huggingface/datasets/*` 

@thomwolf @mariamabarham @lhoestq 

Comments:
Super nice, good job @patrickvonplaten!
[Dataset scripts] add all datasets scripts

As mentioned, we can have the canonical datasets in the master. For now I also want to include all the data as present on S3 to make the synchronization easier when uploading new datastes. 

@mariamabarham @lhoestq @thomwolf - what do you think? 

If this is ok for you, I can sync up the master with the `add_dataset` branch: https://github.com/huggingface/nlp/pull/37 so that master is up to date. 

Comments:
Update to simplify some datasets conversion

This PR updates the encoding of `Values` like `integers`, `boolean` and `float` to use python casting and avoid having to cast in the dataset scripts, as mentioned here: https://github.com/huggingface/nlp/pull/37#discussion_r420176626

We could also change (not included in this PR yet):
- `supervized_keys` to make them a NamedTuple instead of a dataclass, and
- handle specifically the `Translation` features.
as mentioned here: https://github.com/huggingface/nlp/pull/37#discussion_r421740236

@patrickvonplaten @mariamabarham tell me if you want these two last changes as well.

Comments:
Awesome! 

Also we should convert `tf.io.gfile.exists` into `os.path.exists` , `tf.io.gfile.listdir`into `os.listdir` and `tf.io.gfile.glob`  into `glob.glob` (will need to add `import glob`)

> Also we should convert `tf.io.gfile.exists` into `os.path.exists` , `tf.io.gfile.listdir`into `os.listdir` and `tf.io.gfile.glob` into `glob.glob` (will need to add `import glob`)

We should probably open a new PR about this

I think it might be a good idea to both change the supervised keys to a named tuple and also handle the translation features specifically.

Just noticed that `pyarrow` apparently does not have a `is_boolean` function. Or do I have the wrong `pyarrow` version? 

Ah, it was a typo `pa.types.is_boolean` is the correct name. Will fix in: https://github.com/huggingface/nlp/pull/59
Better cached path

### Changes:
- The `cached_path` no longer returns None if the file is missing/the url doesn't work. Instead, it can raise `FileNotFoundError` (missing file), `ConnectionError` (no cache and unreachable url) or `ValueError` (parsing error)
- Fix requests to firebase API that doesn't handle HEAD requests...
- Allow custom download in datasets script: it allows to use `tf.io.gfile.copy` for example, to download from google storage. I added an example: the `boolq` script

Comments:
I should have read this PR before doing my own: https://github.com/huggingface/nlp/pull/62 :D 
will close mine. Looks great :-) 

> Awesome, this is really nice!
> 
> By the way, we should improve the `cached_path` method of the `transformers` repo similarly, don't you think (@patrickvonplaten in particular).

Yeah, we should do the same in `transformers` I think - will note it down.
[Testing] Improved testing structure

This PR refactors the test design a bit and puts the mock download manager in the `utils` files as it is just a test helper class.

as @mariamabarham pointed out, creating a dummy folder structure can be quite hard to grasp.
This PR tries to change that to some extent.

It follows the following logic for the `dummy` folder structure now:
1.) The data bulider has no config -> the  `dummy` folder structure is:
`dummy/<version>/dummy_data.zip`
2) The data builder has >= 1 configs -> the `dummy` folder structure is: 
`dummy/<config_name_1>/<version>/dummy_data.zip`
`dummy/<config_name_2>/<version>/dummy_data.zip`

Now, the difficult part is how to create the `dummy_data.zip` file. There are two cases:
A) The `data_urs` parameter inserted into the `download_and_extract` fn is a **string**:
-> the `dummy_data.zip` file zips the folder: 
`dummy_data/<relative_path_of_folder_structure_of_url>`
B) The `data_urs` parameter inserted into the `download_and_extract` fn is a **dict**:
-> the `dummy_data.zip` file zips the folder: 
`dummy_data/<relative_path_of_folder_structure_of_url_behind _key_1>`
`dummy_data/<relative_path_of_folder_structure_of_url_behind _key_2>`

By relative folder structure I mean `url_path.split('./')[-1]`. As an example the dataset **xquad** by deepmind has the following url path behind the key `de`: `https://github.com/deepmind/xquad/blob/master/xquad.de.json` 
-> This means that the relative url path should be `xquad.de.json`.


@mariamabarham B) is a change from how is was before and I think is makes more sense. 
While before the `dummy_data.zip` file for xquad with config `de` looked like:
`dummy_data/de` it would now look like `dummy_data/xquad.de.json`. I think this is better and easier to understand. 

Therefore there are currently 6 tests that would have to have changed their dummy folder structure, but which can easily be done (30min). 

I also added a function: `print_dummy_data_folder_structure` that prints out the expected structures when testing which should be quite helpful.

Comments:
Awesome!
Let's have this in the doc at the end :-)
[Checksums] If no configs exist prevent to run over empty list

`movie_rationales` e.g. has no configs.

Comments:
Whoops I fixed it directly on master before checking that you have done it in this PR. We may close it

Yeah, I saw :-) But I think we should add this as well since some datasets have an empty list of configs and then as the code is now it would fail. 

In this PR, I just make sure that the code jumps in the correct else if "there are no configs" as is the case for some datasets @mariamabarham 

Sorry, I thought you meant a different commit . Just saw this one: https://github.com/huggingface/nlp/commit/7c644f284e2303b57612a6e7c904fe13906d893f
.

All good then :-)
[Checksums] Error for some datasets

The checksums command works very nicely for `squad`. But for `crime_and_punish` and `xnli`, 
the same bug happens:

When running: 
```
python nlp-cli nlp-cli test xnli --save_checksums
```

leads to:

```
  File "nlp-cli", line 33, in <module>
    service.run()
  File "/home/patrick/python_bin/nlp/commands/test.py", line 61, in run
    ignore_checksums=self._ignore_checksums,
  File "/home/patrick/python_bin/nlp/builder.py", line 383, in download_and_prepare
    self._download_and_prepare(dl_manager=dl_manager, download_config=download_config)
  File "/home/patrick/python_bin/nlp/builder.py", line 627, in _download_and_prepare
    dl_manager=dl_manager, max_examples_per_split=download_config.max_examples_per_split,
  File "/home/patrick/python_bin/nlp/builder.py", line 431, in _download_and_prepare
    split_generators = self._split_generators(dl_manager, **split_generators_kwargs)
  File "/home/patrick/python_bin/nlp/datasets/xnli/8bf4185a2da1ef2a523186dd660d9adcf0946189e7fa5942ea31c63c07b68a7f/xnli.py", line 95, in _split_generators
    dl_dir = dl_manager.download_and_extract(_DATA_URL)
  File "/home/patrick/python_bin/nlp/utils/download_manager.py", line 246, in download_and_extract
    return self.extract(self.download(url_or_urls))
  File "/home/patrick/python_bin/nlp/utils/download_manager.py", line 186, in download
    self._record_sizes_checksums(url_or_urls, downloaded_path_or_paths)
  File "/home/patrick/python_bin/nlp/utils/download_manager.py", line 166, in _record_sizes_checksums
    self._recorded_sizes_checksums[url] = get_size_checksum(path)
  File "/home/patrick/python_bin/nlp/utils/checksums_utils.py", line 81, in get_size_checksum
    with open(path, "rb") as f:
TypeError: expected str, bytes or os.PathLike object, not tuple
```


Comments:
@lhoestq - could you take a look? It's not very urgent though!

Fixed with 06882b4

Now your command works :)
Note that you can also do
```
nlp-cli test datasets/nlp/xnli --save_checksums
```
So that it will save the checksums directly in the right directory.

Awesome!
[Tests] add slow tests

This PR adds a slow test that downloads the "real" dataset. The test is decorated as "slow" so that it will not automatically run on circle ci.

Before uploading a dataset, one should test that this test passes, manually by running 

```
RUN_SLOW=1 pytest tests/test_dataset_common.py::DatasetTest::test_load_real_dataset_<your-dataset-script-name>
```

This PR should be merged after PR: #33 

Comments:
Fix map caching notebooks

Previously, caching results with `.map()` didn't work in notebooks.
To reuse a result, `.map()` serializes the functions with `dill.dumps` and then it hashes it.

The problem is that when using `dill.dumps` to serialize a function, it also saves its origin (filename + line no.) and the origin of all the `globals` this function needs. However for notebooks and shells, the filename looks like \<ipython-input-13-9ed2afe61d25\> and the line no. changes often.

To fix the problem, I added a new dispatch function for code objects that ignore the origin of the code if it comes from a notebook or a python shell.

I tested these cases in a notebook:
- lambda functions
- named functions
- methods
- classmethods
- staticmethods
- classes that implement `__call__`

The caching now works as expected for all of them :)
I also tested the caching in the demo notebook and it works fine !

Comments:
[Circle ci] Install a virtual env before running tests

Install a virtual env before running tests to not running into sudo issues when dynamically downloading files. 

Same number of tests now pass / fail as on my local computer: 
![Screenshot from 2020-05-01 12-14-44](https://user-images.githubusercontent.com/23423619/80798814-8a0a0a80-8ba5-11ea-8db8-599d33bbfccd.png)



Comments:
add metrics which require download files from github

To download files from github, I copied the `load_dataset_module` and its dependencies (without the builder) in `load.py` to `metrics/metric_utils.py`. I made the following changes:

- copy the needed files in a folder`metric_name` 
- delete all other files that are not needed

For metrics that require an external import, I first create a `<metric_name>_imports.py` file which contains all external urls. Then I create a `<metric_name>.py` in which I will load the external files using `<metric_name>_imports.py` 

Comments:
[Tests] Clean tests

the abseil testing library (https://abseil.io/docs/python/quickstart.html) is better than the one I had before, so I decided to switch to that and changed the `setup.py` config file. 
Abseil has more support and a cleaner API for parametrized testing I think. 

I added a list of all dataset scripts that are currently on AWS, but will replace that once the 
API is integrated into this lib. 

One can now easily test for just a single function for a single dataset with:
`tests/test_dataset_common.py::DatasetTest::test_load_dataset_wikipedia` 

NOTE: This PR is rebased on PR #29 so should be merged after.

Comments:
Add metrics

This PR is a draft for adding metrics (sacrebleu and seqeval are added)

use case examples:
`import nlp`
**sacrebleu:**
```
refs = [['The dog bit the man.', 'It was not unexpected.', 'The man bit him first.'],
        ['The dog had bit the man.', 'No one was surprised.', 'The man had bitten the dog.']]
sys = ['The dog bit the man.', "It wasn't surprising.", 'The man had just bitten him.']
sacrebleu = nlp.load_metrics('sacrebleu')
print(sacrebleu.score)
```

**seqeval:**
```
y_true = [['O', 'O', 'O', 'B-MISC', 'I-MISC', 'I-MISC', 'O'], ['B-PER', 'I-PER', 'O']]
y_pred = [['O', 'O', 'B-MISC', 'I-MISC', 'I-MISC', 'I-MISC', 'O'], ['B-PER', 'I-PER', 'O']]
seqeval = nlp.load_metrics('seqeval')
print(seqeval.accuracy_score(y_true, y_pred)
print(seqeval.f1_score(y_true, y_pred)
```
_examples are taken from the corresponding web page_

your comments and suggestions are more than welcomed





Comments:
adding bleu score code

this PR add the BLEU score metric to the lib. It can be tested by running the following code.

` from nlp.metrics import bleu

hyp1 = "It is a guide to action which ensures that the military always obeys the commands of the party"
ref1a = "It is a guide to action that ensures that the military  forces always being under the commands of the party "
 ref1b = "It is the guiding principle which guarantees the military force always being under the command of the Party"
ref1c = "It is the practical guide for the army always to heed the directions of the party"
    
list_of_references = [[ref1a, ref1b, ref1c]]
hypotheses = [hyp1]
bleu = bleu.bleu_score(list_of_references, hypotheses,4, smooth=True)
print(bleu) `

Comments:
Cleanup Features - Updating convert command - Fix Download manager

This PR makes a number of changes:

# Updating `Features`

Features are a complex mechanism provided in `tfds` to be able to modify a dataset on-the-fly when serializing to disk and when loading from disk.

We don't really need this because (1) it hides too much from the user and (2) our datatype can be directly mapped to Arrow tables on drive so we usually don't need to change the format before/after serialization.

This PR extracts and refactors these features in a single `features.py` files. It still keep a number of features classes for easy compatibility with tfds, namely the `Sequence`, `Tensor`, `ClassLabel` and `Translation` features.

Some more complex features involving a pre-processing on-the-fly during serialization are kept:
- `ClassLabel` which are able to convert from label strings to integers,
- `Translation`which does some check on the languages.

# Updating the `convert` command

We do a few updates here
- following the simplification of the `features` (cf above), conversion are updated
- we also makes it simpler to convert a single file
- some code need to be fixed manually after conversion (e.g. to remove some encoding processing in former tfds `Text` features. We highlight this code with a "git merge conflict" style syntax for easy manual fixing.

# Fix download manager iterator

You kept me up quite late on Tuesday night with this `os.scandir` change @lhoestq ;-)


Comments:
For conflicts, I think the mention hint "This should be modified because it mentions ..." is missing.

Looks great!
Replace tf.constant for TF

Replace simple tf.constant type of Tensor to tf.ragged.constant which allows to have examples of different size in a tf.data.Dataset.

Now the training works with TF. Here the same example than for the PT in collab:

```python
import tensorflow as tf
import nlp
from transformers import BertTokenizerFast, TFBertForQuestionAnswering

# Load our training dataset and tokenizer
train_dataset = nlp.load('squad', split="train[:1%]")
tokenizer = BertTokenizerFast.from_pretrained('bert-base-cased')

def get_correct_alignement(context, answer):
    start_idx = answer['answer_start'][0]
    text = answer['text'][0]
    end_idx = start_idx + len(text)
    if context[start_idx:end_idx] == text:
        return start_idx, end_idx       # When the gold label position is good
    elif context[start_idx-1:end_idx-1] == text:
        return start_idx-1, end_idx-1   # When the gold label is off by one character
    elif context[start_idx-2:end_idx-2] == text:
        return start_idx-2, end_idx-2   # When the gold label is off by two character
    else:
        raise ValueError()

# Tokenize our training dataset
def convert_to_features(example_batch):
    # Tokenize contexts and questions (as pairs of inputs)
    input_pairs = list(zip(example_batch['context'], example_batch['question']))
    encodings = tokenizer.batch_encode_plus(input_pairs, pad_to_max_length=True)

    # Compute start and end tokens for labels using Transformers's fast tokenizers alignement methods.
    start_positions, end_positions = [], []
    for i, (context, answer) in enumerate(zip(example_batch['context'], example_batch['answers'])):
        start_idx, end_idx = get_correct_alignement(context, answer)
        start_positions.append([encodings.char_to_token(i, start_idx)])
        end_positions.append([encodings.char_to_token(i, end_idx-1)])
    
    if start_positions and end_positions:
      encodings.update({'start_positions': start_positions,
                        'end_positions': end_positions})
    return encodings

train_dataset = train_dataset.map(convert_to_features, batched=True)

columns = ['input_ids', 'token_type_ids', 'attention_mask', 'start_positions', 'end_positions']
train_dataset.set_format(type='tensorflow', columns=columns)
features = {x: train_dataset[x] for x in columns[:3]} 
labels = {"output_1": train_dataset["start_positions"]}
labels["output_2"] = train_dataset["end_positions"]
tfdataset = tf.data.Dataset.from_tensor_slices((features, labels)).batch(8)
model = TFBertForQuestionAnswering.from_pretrained("bert-base-cased")
loss_fn = tf.keras.losses.SparseCategoricalCrossentropy(reduction=tf.keras.losses.Reduction.NONE, from_logits=True)
opt = tf.keras.optimizers.Adam(learning_rate=3e-5)
model.compile(optimizer=opt,
              loss={'output_1': loss_fn, 'output_2': loss_fn},
              loss_weights={'output_1': 1., 'output_2': 1.},
              metrics=['accuracy'])
model.fit(tfdataset, epochs=1, steps_per_epoch=3)
```

Comments:
Awesome!
Updating caching mechanism - Allow dependency in dataset processing scripts - Fix style and quality in the repo

This PR has a lot of content (might be hard to review, sorry, in particular because I fixed the style in the repo at the same time).

# Style & quality:
You can now install the style and quality tools with `pip install -e .[quality]`. This will install black, the compatible version of sort and flake8.
You can then clean the style and check the quality before merging your PR with:
```bash
make style
make quality
```

# Allow dependencies in dataset processing scripts
We can now allow (some level) of imports in dataset processing scripts (in addition to PyPi imports).
Namely, you can do the two following things:

Import from a relative path to a file in the same folder as the dataset processing script:
```python
import .c4_utils 
``` 

Or import from a relative path to a file in a folder/archive/github repo to which you provide an URL after the import state with `# From: [URL]`:
```python
import .clicr.dataset_code.build_json_dataset  # From: https://github.com/clips/clicr
```

In both these cases, after downloading the main dataset processing script, we will identify the location of these dependencies, download them and copy them in the dataset processing script folder.

Note that only direct import in the dataset processing script will be handled.
We don't recursively explore the additional import to download further files.
Also, when we download from an additional directory (in the second case above), we recursively add `__init__.py` to all the sub-folder so you can import from them.

This part is still tested for now. If you've seen datasets which required external utilities, tell me and I can test it.

# Update the cache to have a better local structure

The local structure in the `src/datasets` folder is now: `src/datasets/DATASET_NAME/DATASET_HASH/*`

The hash is computed from the full code of the dataset processing script as well as all the local and downloaded dependencies as mentioned above. This way if you change some code in a utility related to your dataset, a new hash should be computed.

Comments:
LGTM
create our own DownloadManager

I tried to create our own - and way simpler - download manager, by replacing all the complicated stuff with our own `cached_path` solution.
With this implementation, I tried `dataset = nlp.load('squad')` and it seems to work fine.

For the implementation, what I did exactly:
- I copied the old download manager
- I removed all the dependences to the old `download` files
- I replaced all the download + extract calls by calls to `cached_path`
- I removed unused parameters (extract_dir, compute_stats) (maybe compute_stats could be re-added later if we want to compute stats...)
- I left some functions unimplemented for now. We will probably have to implement them because they are used by some datasets scripts (download_kaggle_data, iter_archive) or because we may need them at some point (download_checksums, _record_sizes_checksums)

Let me know if you think that this is going the right direction or if you have remarks.
Note: I didn't write any test yet as I wanted to read your remarks first

Comments:
Looks great to me! 

The new download manager is ready. I removed the old folder and I fixed a few remaining dependencies.
I tested it on squad and a few others from the dataset folder and it works fine.

The only impact of these changes is that it breaks the `download_and_prepare` script that was used to register the checksums when we create a dataset, as the checksum logic is not implemented.

Let me know if you have remarks

Ok merged it (a bit fast for you to update the copyright, now I see that. but it's ok, we'll do a pass on these doc/copyright before releasing anyway)

Actually two additional things here @lhoestq (I merged too fast sorry, let's make a new PR for additional developments):
- I think we can remove some dependencies now (e.g. `promises`) in setup.py, can you have a look?
- also, I think we can remove the boto3 dependency like here: https://github.com/huggingface/transformers/pull/3968
[Make style]

Added Makefile and applied make style to all. 
make style runs the following code:

```
style:
          black --line-length 119 --target-version py35 src
          isort --recursive src
```

It's the same code that is run in `transformers`.

Comments:
I think this can be quickly reproduced. 
I use `black, version 19.10b0`. 

When running: 
`black nlp/src/arrow_reader.py` 
it gives me: 

```
error: cannot format /home/patrick/hugging_face/nlp/src/nlp/arrow_reader.py: cannot use --safe with this file; failed to parse source file.  AST error message: invalid syntax (<unknown>, line 78)
Oh no! 💥 💔 💥
1 file failed to reformat.
```

The line in question is: 
https://github.com/huggingface/nlp/blob/6922a16705e61f9e31a365f2606090b84d49241f/src/nlp/arrow_reader.py#L78

What is weird is that the trainer file in `transformers` has more or less the same syntax and black does not fail there: 
https://github.com/huggingface/transformers/blob/cb3c2212c79d7ff0a4a4e84c3db48371ecc1c15d/src/transformers/trainer.py#L95

I googled quite a bit about black & typing hints yesterday and didn't find anything useful. 
Any ideas @thomwolf @julien-c @LysandreJik ?

> I think this can be quickly reproduced.
> I use `black, version 19.10b0`.
> 
> When running:
> `black nlp/src/arrow_reader.py`
> it gives me:
> 
> ```
> error: cannot format /home/patrick/hugging_face/nlp/src/nlp/arrow_reader.py: cannot use --safe with this file; failed to parse source file.  AST error message: invalid syntax (<unknown>, line 78)
> Oh no! 💥 💔 💥
> 1 file failed to reformat.
> ```
> 
> The line in question is:
> https://github.com/huggingface/nlp/blob/6922a16705e61f9e31a365f2606090b84d49241f/src/nlp/arrow_reader.py#L78
> 
> What is weird is that the trainer file in `transformers` has more or less the same syntax and black does not fail there:
> https://github.com/huggingface/transformers/blob/cb3c2212c79d7ff0a4a4e84c3db48371ecc1c15d/src/transformers/trainer.py#L95
> 
> I googled quite a bit about black & typing hints yesterday and didn't find anything useful.
> Any ideas @thomwolf @julien-c @LysandreJik ?

Ok I found the problem. It was the one Julien mentioned and has nothing to do with this line. Black's error message is a bit misleading here, I guess

Ok, just had to remove the python 2 syntax comments `# type`. 

Good to merge for me now @thomwolf
[Map Function] add assert statement if map function does not return dict or None

IMO, if a function is provided that is not a print statement (-> returns variable of type `None`) or a function that updates the datasets (-> returns variable of type `dict`), then a `TypeError` should be raised. 

Not sure whether you had cases in mind where the user should do something else @thomwolf , but I think a lot of silent errors can be avoided with this assert statement.

Comments:
Also added to an assert statement that if a dict is returned by function, all values of `dicts` are `lists`

Wait to merge until `make style` is set in place.

Updated the assert statements. Played around with multiple cases and it should be good now IMO.
[Clean up] Datasets

Clean up `nlp/datasets` folder. 

As I understood, eventually the `nlp/datasets` shall not exist anymore at all. 

The folder `nlp/datasets/nlp` is kept for the moment, but won't be needed in the future, since it will live on S3 (actually it already does) at: `https://s3.console.aws.amazon.com/s3/buckets/datasets.huggingface.co/nlp/?region=us-east-1` and the different `dataset downloader scripts will be added to `nlp/src/nlp` when downloaded by the user. 

The folder `nlp/datasets/checksums` is kept for now, but won't be needed anymore in the future. 

The remaining folders/ files are leftovers from tensorflow-datasets and are not needed. The can be looked up in the private tensorflow-dataset repo.

Comments:
Yes!