NyssaW/github-issues huggingface dataset:
CompGuessWhat?! 0.2.0

We updated some metadata information associated with the dataset. In addition, we've updated the `create_dummy_data.py` script to generate data samples for the dataset. 

Comments:
I don't see any significant change in the dataset script (except the version value update), can you check that again please ?

Hi @aleSuglia , can you check that all the changes you wanted to do are in the dataset script ?

Hey sorry but I'm in the middle of a conference deadline. I'll let you know asap!

Ok np :)
Good luck with your work for the conference

I finally managed to find some time to complete this. The only weird thing about this release is that I had to run the tests with the ignore checksum flag. Could it be because the Dropbox link doesn't change but the file does? Sorry didn't have the time to check the code to see what's happening behind the scenes.


Yes if the file changed, then the checksum verification won't pass as it expects to see the checksum of the old file.
The checksum is computed by hashing the complete file.
You can update the checksum by doing 

```
nlp-cli test ./datasets/compguesswhat --save_infos --all_configs
```

Any updates on this?

Hi :)

I think what's left to do is
1- rebase from master, since we changed the name of the library
2- update the metadata file of the dataset using the command 
```
datasets-cli test ./datasets/compguesswhat --save_infos --all_configs --ignore_verifications
```

This command should update the checksum of the dropbox file

That's perfect. I'll have a look at it later today!

Nice thanks !

@lhoestq not sure why the quality check doesn't pass. Unfortunately CircleCI doesn't show the actual error. If I run `black` on my machine it works just fine. Ideas?

@lhoestq any updates? :) 

Your version of `black` might be outdated, or you run using `black` instead of `make style` since it reformatted 100+ files.
Could you try to update black, then `make style` ?

Yes I think my versions of isort and black were outdated. Thanks @lhoestq :)


It still doesn't look right in terms of line-length.
Are you running `black` or `make style` ?

I'm running `make style`. This is the output of the command:

```
black --line-length 119 --target-version py36 tests src benchmarks datasets metrics
All done! ✨ 🍰 ✨
250 files left unchanged.
isort tests src benchmarks datasets metrics
```

Weird I have the same output without file changes with black `20.8b1` and isort `5.6.4` using `make style` too

I think that's because black doesn't revert the changes you first did with the old version.
Could you open a new PR with only the ComGuessWhat files updated ? Hopefully now that black is up to date it should work directly (and to avoid 100+ files changes)

I will have a look at it tomorrow. Thanks for your help!

I'm closing this one and I'll open a new one.
Fix tokenizers caching

I've found some cases where the caching didn't work properly for tokenizers:

1. if a tokenizer has a regex pattern, then the caching would be inconsistent across sessions
2. if a tokenizer has a cache attribute that changes after some calls, the the caching would not work after cache updates
3. if a tokenizer is used inside a function, the caching of this function would result in the same cache file for different tokenizers
4. if `unique_no_split_tokens`'s attribute is not the same across sessions (after loading a tokenizer) then the caching could be inconsistent

To fix that, this is what I did:

1. register a specific `save_regex` function for pickle that makes regex dumps deterministic
2. ignore cache attribute of some tokenizers before dumping
3. enable recursive dump by default for all dumps
4. make `unique_no_split_tokens` deterministic in https://github.com/huggingface/transformers/pull/6461

I also added tests to make sure that tokenizers hashing works as expected.
In the future we should find a way to test if hashing also works across session (maybe using two CI jobs ? or by hardcoding a tokenizer's hash ?)

Comments:
This should fix #501 and also the issue you sent me on slack @sgugger .
Caching doesn't work for map (non-deterministic)

The caching functionality doesn't work reliably when tokenizing a dataset. Here's a small example to reproduce it. 

```python
import nlp
import transformers

def main():
    ds = nlp.load_dataset("reddit", split="train[:500]")

    tokenizer = transformers.AutoTokenizer.from_pretrained("gpt2")

    def convert_to_features(example_batch):
        input_str = example_batch["body"]
        encodings = tokenizer(input_str, add_special_tokens=True, truncation=True)
        return encodings

    ds = ds.map(convert_to_features, batched=True)

if __name__ == "__main__":
    main()
```

Roughly 3/10 times, this example recomputes the tokenization.

Is this expected behaviour?

Comments:
Thanks for reporting !

To store the cache file, we compute a hash of the function given in `.map`, using our own hashing function.
The hash doesn't seem to stay the same over sessions for the tokenizer.
Apparently this is because of the regex at `tokenizer.pat` is not well supported by our hashing function.

I'm working on a fix

Thanks everyone. Works great now.

Hi. I believe the fix was for the nlp library. Is there a solution to handle compiled regex expressions in .map() with the caching. I want to run a simple regex pattern on a big dataset, but I am running into the issue of compiled expression not being cached. 

Instead of opening a new issue, I thought I would put my query here. Let me know if a new issue would be more suitable. Thanks

Hi @MaveriQ! This fix is also included in the `datasets` library. Can you provide a reproducer?
skip header in PAWS-X

This should fix #485 

I also updated the `dataset_infos.json` file that is used to verify the integrity of the generated splits (the number of examples was reduced by one).

Note that there are new fields in `dataset_infos.json` introduced in the latest release 0.4.0 corresponding to post processing info. I removed them in this case when I ran `nlp-cli ./datasets/xtreme --save_infos` to keep backward compatibility (versions 0.3.0 can't load these fields).

I think I'll change the logic so that `nlp-cli test` doesn't create these fields for dataset with no post processing

Comments:
stack vectors in pytorch and tensorflow

When the format of a dataset is set to pytorch or tensorflow, and if the dataset has vectors in it, they were not stacked together as tensors when calling `dataset[i:i + batch_size][column]` or `dataset[column]`.

I added support for stacked tensors for both pytorch and tensorflow.
For ragged tensors, they are stacked only for tensorflow as pytorch doesn't support ragged tensors.


Comments:
Fix numpy stacking

When getting items using a column name as a key, numpy arrays were not stacked.
I fixed that and added some tests.

There is another issue that still needs to be fixed though: when getting items using a column name as a key, pytorch tensors are not stacked (it outputs a list of tensors). This PR should help with the to fix this issue.

Comments:
This PR also fixed a bug where numpy arrays were returned instead of pytorch tensors when getting with a clumn as a key.
nlp.Features does not distinguish between nullable and non-nullable types in PyArrow schema

Here's the code I'm trying to run:

```python
dset_wikipedia = nlp.load_dataset("wikipedia", "20200501.en", split="train", cache_dir=args.cache_dir)
dset_wikipedia.drop(columns=["title"])
dset_wikipedia.features.pop("title")
dset_books = nlp.load_dataset("bookcorpus", split="train", cache_dir=args.cache_dir)
dset = nlp.concatenate_datasets([dset_wikipedia, dset_books])
```

This fails because they have different schemas, despite having identical features.

```python
assert dset_wikipedia.features == dset_books.features # True
assert dset_wikipedia._data.schema == dset_books._data.schema # False
```

The Wikipedia dataset has 'text: string', while the BookCorpus dataset has 'text: string not null'. Currently I hack together a working schema match with the following line, but it would be better if this was handled in Features themselves.

```python
dset_wikipedia._data = dset_wikipedia.data.cast(dset_books._data.schema)
```


Comments:
In 0.4.0, the assertion in `concatenate_datasets ` is on the features, and not the schema.
Could you try to update `nlp` ?

Also, since 0.4.0, you can use `dset_wikipedia.cast_(dset_books.features)` to avoid the schema cast hack.

Or maybe the assertion comes from elsewhere ?

I'm using the master branch. The assertion failure comes from the underlying `pa.concat_tables()`, which is in the pyarrow package. That method does check schemas.

Since `features.type` does not contain information about nullable vs non-nullable features, the `cast_()` method won't resolve the schema mismatch. There is information in a schema which is not stored in features.

I'm doing a refactor of type inference in #363 . Both text fields should match after that

By default nullable will be set to True

It should be good now. I was able to run

```python
>>> from nlp import concatenate_datasets, load_dataset
>>>
>>> bookcorpus = load_dataset("bookcorpus", split="train")
>>> wiki = load_dataset("wikipedia", "20200501.en", split="train")
>>> wiki.remove_columns_("title")  # only keep the text
>>>
>>> assert bookcorpus.features.type == wiki.features.type
>>> bert_dataset = concatenate_datasets([bookcorpus, wiki])
```

Thanks!
No 0.4.0 release on GitHub

0.4.0 was released on PyPi, but not on GitHub. This means [the documentation](https://huggingface.co/nlp/) is still displaying from 0.3.0, and that there's no tag to easily clone the 0.4.0 version of the repo.

Comments:
I did the release on github, and updated the doc :)
Sorry for the delay

Thanks!
Loading preprocessed Wikipedia dataset requires apache_beam

Running 

`nlp.load_dataset("wikipedia", "20200501.en", split="train", dir="/tmp/wikipedia")`

gives an error if apache_beam is not installed, stemming from

https://github.com/huggingface/nlp/blob/38eb2413de54ee804b0be81781bd65ac4a748ced/src/nlp/builder.py#L981-L988

This succeeded without the dependency in version 0.3.0. This seems like an unnecessary dependency to process some dataset info if you're using the already-preprocessed version. Could it be removed?

Comments:
PAWS dataset first item is header

```
import nlp
dataset = nlp.load_dataset('xtreme', 'PAWS-X.en')
dataset['test'][0]
```

prints the following

```
{'label': 'label', 'sentence1': 'sentence1', 'sentence2': 'sentence2'}
```

dataset['test'][0] should probably be the first item in the dataset, not just a dictionary mapping the column names to themselves. Probably just need to ignore the first row in the dataset by default or something like that.

Comments:
update mirror for RT dataset


Comments:
Thanks for adding this mirror link :)

Could you run the following command to update the json file `dataset_infos.json` used to verify the integrity of the downloaded file ?

```
nlp-cli test ./datasets/rotten_tomatoes --save_infos --ignore_verifications
```

done! @lhoestq 

the build_doc CI fail comes from master and has been fixed on master

done @thomwolf @lhoestq
rotten tomatoes movie review dataset taken down

In an interesting twist of events, the individual who created the movie review seems to have left Cornell, and their webpage has been removed, along with the movie review dataset (http://www.cs.cornell.edu/people/pabo/movie-review-data/rt-polaritydata.tar.gz). It's not downloadable anymore.

Comments:
found a mirror: https://storage.googleapis.com/seldon-datasets/sentence_polarity_v1/rt-polaritydata.tar.gz

fixed in #484 

Closing this one. Thanks again @jxmorris12 for taking care of this :)
Column indexing hotfix

As observed for example in #469 , currently `__getitem__` does not convert the data to the dataset format when indexing by column. This is a hotfix that imitates functional 0.3.0. code. In the future it'd probably be nice to have a test there.

Comments:
Looks good to me as well but we'll want to add a test indeed.
You can add one if you have time @TevenLeScao.
Otherwise, we'll do it when we are back with Quentin. 

I fixed it in #494
add METEOR metric

Added the METEOR metric. Can be used like this:

```python
import nlp
meteor = nlp.load_metric('metrics/meteor')
meteor.compute(["some string", "some string"], ["some string", "some similar string"])
# {'meteor': 0.6411637931034483}
meteor.add("some string", "some string")
meteor.add('some string", "some similar string")
meteor.compute()
# {'meteor': 0.6411637931034483}
```

Uses [NLTK's implementation](https://www.nltk.org/api/nltk.translate.html#module-nltk.translate.meteor_score), [(source)](https://github.com/nltk/nltk/blob/develop/nltk/translate/meteor_score.py)

Comments:
Really nice !
Thanks for adding this one.

I noticed that there are some '-' that are left in the description in the middle of some workds. It migh come from copy-pasting the pdf paper. ex: `im-provement`. Could you fix that please ?

@lhoestq 
Linebreaks have been removed! Note that there are still a few compound words that are hyphenated intentionally. 

I think you just need to rebase from master to fix the CI :)

Yes I made the mistake of simply merging master into this branch. A rebase seems to be neater :) Although all the commits ended up being added twice. I assume you just squash them into a single one on merge anyways?

Yes indeed they'll be squashed
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 

Comments:
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 :)
Overview.ipynb throws exceptions with nlp 0.4.0

with nlp 0.4.0, the TensorFlow example in Overview.ipynb throws the following exceptions:


---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-5-48907f2ad433> in <module>
----> 1 features = {x: train_tf_dataset[x].to_tensor(default_value=0, shape=[None, tokenizer.max_len]) for x in columns[:3]}
      2 labels = {"output_1": train_tf_dataset["start_positions"].to_tensor(default_value=0, shape=[None, 1])}
      3 labels["output_2"] = train_tf_dataset["end_positions"].to_tensor(default_value=0, shape=[None, 1])
      4 tfdataset = tf.data.Dataset.from_tensor_slices((features, labels)).batch(8)

<ipython-input-5-48907f2ad433> in <dictcomp>(.0)
----> 1 features = {x: train_tf_dataset[x].to_tensor(default_value=0, shape=[None, tokenizer.max_len]) for x in columns[:3]}
      2 labels = {"output_1": train_tf_dataset["start_positions"].to_tensor(default_value=0, shape=[None, 1])}
      3 labels["output_2"] = train_tf_dataset["end_positions"].to_tensor(default_value=0, shape=[None, 1])
      4 tfdataset = tf.data.Dataset.from_tensor_slices((features, labels)).batch(8)

AttributeError: 'numpy.ndarray' object has no attribute 'to_tensor'

Comments:
Thanks for reporting this issue

There was a bug where numpy arrays would get returned instead of tensorflow tensors.
This is fixed on master.

I tried to re-run the colab and encountered this error instead:

```
AttributeError: 'tensorflow.python.framework.ops.EagerTensor' object has no attribute 'to_tensor'
```

This is because the dataset returns a Tensor and not a RaggedTensor.
But I think we should always return a RaggedTensor unless the length of the sequence is fixed (it that case they can be stack into a Tensor).

Hi, I got another error (on Colab):

```python
# You can read a few attributes of the datasets before loading them (they are python dataclasses)
from dataclasses import asdict

for key, value in asdict(datasets[6]).items():
    print('👉 ' + key + ': ' + str(value))

---------------------------------------------------------------------------

TypeError                                 Traceback (most recent call last)

<ipython-input-6-b8ace6c227a2> in <module>()
      2 from dataclasses import asdict
      3 
----> 4 for key, value in asdict(datasets[6]).items():
      5     print('👉 ' + key + ': ' + str(value))

/usr/local/lib/python3.6/dist-packages/dataclasses.py in asdict(obj, dict_factory)
   1008     """
   1009     if not _is_dataclass_instance(obj):
-> 1010         raise TypeError("asdict() should be called on dataclass instances")
   1011     return _asdict_inner(obj, dict_factory)
   1012 

TypeError: asdict() should be called on dataclass instances
```

Indeed we'll update the cola with the new release coming up this week.
CheckList

Sorry for the large pull request.
- Added checklists as datasets. I can't run `test_load_real_dataset` (see #474), but I can load the datasets successfully as shown in the example notebook
- Added a checklist wrapper 

Comments:
> Also, a little out of my depth there, but would there be a way to have the default pip install checklist command not require mysql and mariadb to be installed? Feels like that might be a source of confusion for users.

I removed the pattern dependency, mysql is not a requirement anymore. I'm not sure where mariadb is coming from. 

Thanks for your contribution, @marcotcr. Are you still interested in adding this dataset?

We are removing the dataset scripts from this GitHub repo and moving them to the Hugging Face Hub: https://huggingface.co/datasets

We would suggest you create this dataset there. Please, feel free to tell us if you need some help.
misc. bugs and quality of life

A few misc. bugs and QOL improvements that I've come across in using the library. Let me know if you don't like any of them and I can adjust/remove them.

1. Printing datasets without a description field throws an error when formatting the `single_line_description`. This fixes that, and also adds some formatting to the repr to make it slightly more readable.
```
>>> print(list_datasets()[0])
nlp.ObjectInfo(
	id='aeslc',
	description='A collection of email messages of employees in the Enron Corporation.There are two features:  - email_body: email body text.  - subject_line: email subject text.',
	files=[nlp.S3Object('aeslc.py'), nlp.S3Object('dataset_infos.json'), nlp.S3Object('dummy/1.0.0/dummy_data-zip-extracted/dummy_data/AESLC-master/enron_subject_line/dev/allen-p_inbox_29.subject'), nlp.S3Object('dummy/1.0.0/dummy_data-zip-extracted/dummy_data/AESLC-master/enron_subject_line/test/allen-p_inbox_24.subject'), nlp.S3Object('dummy/1.0.0/dummy_data-zip-extracted/dummy_data/AESLC-master/enron_subject_line/train/allen-p_inbox_20.subject'), nlp.S3Object('dummy/1.0.0/dummy_data.zip'), nlp.S3Object('urls_checksums/checksums.txt')]
)
```

2. Add id-only option to `list_datasets` and `list_metrics` to allow the user to easily print out just the names of the datasets & metrics. I often found myself annoyed that this took so many strokes to do.

```python
[dataset.id for dataset in list_datasets()] # before
list_datasets(id_only=True) # after
```

3. Fix null-seed randomization caching. When using `train_test_split` and `shuffle`, the computation was being cached even without a seed or generator being passed. The result was that calling `.shuffle` more than once on the same dataset didn't do anything without passing a distinct seed or generator. Likewise with `train_test_split`.

4. Indexing by iterables of bool. I added support for passing an iterable of type bool to `_getitem` as a numpy/pandas-like indexing method. Let me know if you think it's redundant with `filter` (I know it's not optimal memory-wise), but I think it's nice to have as a lightweight alternative to do simple things without having to create a copy of the entire dataset, e.g.

```python
dataset[dataset['label'] == 0] # numpy-like bool indexing to look at instances with labels of 0
```

5. Add an `input_column` argument to `map` and `filter`, which allows you to filter/map on a particular column rather than passing the whole dict to the function. Also adds `fn_kwargs` to be passed to the function. I think these together make mapping much cleaner in many cases such as mono-column tokenization:

```python
# before
dataset = dataset.map(lambda batch: tokenizer(batch["text"])
# after
dataset = dataset.map(tokenizer, input_column="text")
dataset = dataset.map(tokenizer, input_column="text", fn_kwargs={"truncation": True, "padding": True})
```

Comments:
Cool thanks, I made those changes. LMK if you think it's ready for merge.

Ok to merge for me
test_load_real_dataset when config has BUILDER_CONFIGS that matter

It a dataset has custom `BUILDER_CONFIGS` with non-keyword arguments (or keyword arguments with non default values), the config is not loaded during the test and causes an error.
I think the problem is that `test_load_real_dataset` calls `load_dataset` with `data_dir=temp_data_dir` ([here](https://github.com/huggingface/nlp/blob/master/tests/test_dataset_common.py#L200)). This causes [this line](https://github.com/huggingface/nlp/blob/master/src/nlp/builder.py#L201) to always be false because `config_kwargs` is not `None`. [This line](https://github.com/huggingface/nlp/blob/master/src/nlp/builder.py#L222) will be run instead, which doesn't use `BUILDER_CONFIGS`.

For an example, you can try running the test for lince:
` RUN_SLOW=1 pytest tests/test_dataset_common.py::LocalDatasetTest::test_load_real_dataset_lince`
which yields
> E           TypeError: __init__() missing 3 required positional arguments: 'colnames', 'classes', and 'label_column'

Comments:
The `data_dir` parameter has been removed. Now the error is `ValueError: Config name is missing`

As mentioned in #470 I think we can have one test with the first config of BUILDER_CONFIGS, and another test that runs all of the configs in BUILDER_CONFIGS

This was fixed in #527 

Closing this one, but feel free to re-open if you have other questions
Adding IWSLT 2017  dataset.

Created a [IWSLT 2017](https://sites.google.com/site/iwsltevaluation2017/TED-tasks) dataset script for the *multilingual data*.

```
Bilingual data:  {Arabic, German, French, Japanese, Korean, Chinese} <-> English
Multilingual data:   German, English, Italian, Dutch, Romanian. (Any pair)
```

I'm unsure how to handle bilingual vs multilingual. Given `nlp` architecture a Config option seems to be the way to go, however, it might be a bit confusing to have different language pairs with different option. Using just language pairs is not viable as English to German exists in both.

Any opinion on how that should be done ?
EDIT: I decided to just omit de-en from multilingual as it's only a subset of the bilingual one. That way only language pairs exist.
EDIT : Could be interesting for #438 

Comments:
Ok I tried to add the dummy dataset (I actually modified the dummy_data command to generate them for me because it was too painful to do that manually).

The dummy_data test seems to work:
```bash
RUN_SLOW=1 pytest tests/test_dataset_common.py::LocalDatasetTest::test_load_dataset_all_configs_iwslt2017
```

However the test on the full data fails, because the `**config_kwargs` don't include `pair, multilingual`.
I could add a default parameter for the Config (but that feels broken, how can one config be the "default" ?). If I do I still have errors, saying that something within the downloader is a directory so I'm not sure where that comes from.

I can share my auto_zip dummy data code if you want (I tried to keep it clean). [Edit: it's [here](https://github.com/Narsil/nlp/tree/auto_zip)]. 
The way it works is that it just keeps X line from the beginning of the original files, and Y lines at the end. It's good enough for my usage, but I guess it could work for most data files out there (as long as they're real text and not binary format)

The slow test doesn't support dataset that require config parameters that don't have default values.

To improve that we can replace it by two tests:
- one test that loads the default config (it can simply be the first config of the config lists for example)
- one tests that iterate over all configs and load them all one by one

By using the configs inside the builder config lists, there is no need to instantiate new configs, so the missing parameter error doesn't happen.

Does that sound good to you ?

Seems fair.
However I'm unsure what I should do ?

Should I wait for #527 to pass and rebase and the command will be the same ?
Should I update something ?

I think everything is fine on your side. Thanks for adding this dataset :)

I think it's better to wait for the slow test to be updated if you don't mind.


Sure ! :)

Thanks for fixing the isort/black changes :)
Feel free to merge if it's good for you @Narsil
[METRICS] Various improvements on metrics

- Disallow the use of positional arguments to avoid `predictions` vs `references` mistakes
- Allow to directly feed numpy/pytorch/tensorflow/pandas objects in metrics

Comments:
The cast function is now called inside `features.encode_example`.
I also added `encode_batch` that was missing.

Moreover I used the cast function in `Dataset.map` to support torch/tensorflow tensors or numpy arrays inputs.

There are tests for tensors inputs in metrics and in .map

I think we can merge
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.

Comments:
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!
Add dataset/mlsum

New pull request that should correct the previous errors. 

The load_real_data stills fails because it is looking for a default dataset URL that does not exists, this does not happen when loading the dataset with load_dataset

Comments:
I think the problem is related to `wiki_dpr` dataset which is making the circle CI failed as you can see:
```
FAILED tests/test_dataset_common.py::AWSDatasetTest::test_load_dataset_wiki_dpr
FAILED tests/test_hf_gcp.py::TestDatasetOnHfGcp::test_script_synced_with_s3_wiki_dpr/dummy_psgs_w100_no_embeddings
FAILED tests/test_hf_gcp.py::TestDatasetOnHfGcp::test_script_synced_with_s3_wiki_dpr/dummy_psgs_w100_with_nq_embeddings
FAILED tests/test_hf_gcp.py::TestDatasetOnHfGcp::test_script_synced_with_s3_wiki_dpr/psgs_w100_no_embeddings
FAILED tests/test_hf_gcp.py::TestDatasetOnHfGcp::test_script_synced_with_s3_wiki_dpr/psgs_w100_with_nq_embeddings

```
I'm facing the same issues with my last commits, I tried to rebase from master but it still not working. Maybe @lhoestq can help with.

Hello, I am confused about the next steps I need to do. Did the forced merge solve the issue ?

Hello :)
I think you can just rebase from master and it should solve the CI error
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.

Comments:
Thanks @TevenLeScao for finding this issue

Thanks @lhoestq for catching this ❤️
[Breaking] Update Dataset and DatasetDict API

This PR contains a few breaking changes so it's probably good to keep it for the next (major) release:
- rename the `flatten`, `drop` and `dictionary_encode_column` methods in `flatten_`, `drop_` and `dictionary_encode_column_` to indicate that these methods have in-place effects as discussed in #166. From now on we should keep the convention of having a trailing underscore for methods which have an in-place effet. I also adopt the conversion of not returning the (self) dataset for these methods. This is different than what PyTorch does for instance (`model.to()` is in-place but return the self model) but I feel like it's a safer approach in terms of UX.
- remove the `dataset.columns` property which returns a low-level Apache Arrow object and should not be used by users. Similarly, remove `dataset. nbytes` which we don't really want to expose in this bare-bone format.
- add a few more properties and methods to `DatasetDict`

Comments:
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

Comments:
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.
Guardian authorship dataset

A new dataset: Guardian news articles for authorship attribution

**tests passed:**
python nlp-cli dummy_data datasets/guardian_authorship --save_infos --all_configs

RUN_SLOW=1 pytest tests/test_dataset_common.py::LocalDatasetTest::test_load_dataset_all_configs_guardian_authorship

**Tests failed:**
Real data: RUN_SLOW=1 pytest tests/test_dataset_common.py::LocalDatasetTest::test_load_real_dataset_guardian_authorship
output: __init__() missing 3 required positional arguments: 'train_folder', 'valid_folder', and 'tes...' 

Remarks: This is the init function of my class. I am not sure why it passes in both my tests and with nlp-cli, but fails here. By the way, I ran this command with another 2 datasets and they failed:
* _glue - OSError: Cannot find data file.
*_newsgroup - FileNotFoundError: Local file datasets/newsgroup/dummy/18828_comp.graphics/3.0.0/dummy_data.zip doesn't exist

Thank you for letting us contribute to such a huge and important library! 

EDIT:
I was able to fix the dummy_data issue. This dataset has around 14 configurations. I was testing with only 2, but their versions were not in a sequence, they were V1.0.0 and V.12.0.0. It seems that the testing code generates testes for all the versions from 0 to MAX, and was testing for versions (and dummy_data.zip files) that do not exist. I fixed that by changing the versions to 1 and 2.



Comments:
Hi ! Glad you managed to fix the version issue.

The command `
python nlp-cli dummy_data datasets/guardian_authorship --save_infos --all_configs` is supposed to generate a json file `dataset_infos.json` next to your dataset script, but I can't see it in the PR.
Can you make sure you have the json file on your side and that you have pushed it ?

Done!

Is there anything else that I should do? and would the new dataset be available via the NLP package now?  

Sorry I forgot to merge this one ! Doing it now

Thanks for the heads up ;)

No worries, this is my first contribution to an online package, and I feel very proud it's part of this library :) Thank you very much!
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 

Comments:
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 reuters21578 dataset

This PR adds the `Reuters_21578` dataset https://kdd.ics.uci.edu/databases/reuters21578/reuters21578.html  
#353 

The datasets is a lit of `.sgm` files which are a bit different from xml file indeed `xml.etree` couldn't be used to read files. I consider them as text file (to avoid using external library)  and read line by line (maybe there is a better way to do, happy to get your opinion on it)

In the Readme file 3 ways to split the dataset are given.:

- The Modified Lewis ("ModLewis") Split: train, test and unused-set

- The Modified Apte ("ModApte") Split : train, test and unused-set

- The Modified Hayes ("ModHayes") Split: train and test

Here I consider the last one as the readme file highlight that this split provides the ability to compare results with those of the 2 first splits.


Comments:
> Awesome !
> Good job on parsing these files :O
> 
> Do you think it would be hard to get the two other split configurations ?

It shouldn't be that hard, I think I can consider different config names for each split 

> > Awesome !
> > Good job on parsing these files :O
> > Do you think it would be hard to get the two other split configurations ?
> 
> It shouldn't be that hard, I think I can consider different config names for each split

Yes that would be perfect

closing this PR and opening a new one to fix the circle CI problems
DEFAULT_TOKENIZER import error in sacrebleu

Latest Version 0.3.0

When loading the metric "sacrebleu" there is an import error due to the wrong path
![image](https://user-images.githubusercontent.com/5303103/88633063-2c5e5f00-d0bd-11ea-8ca8-4704dc975433.png)


Comments:
This issue was resolved by #447
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?


Comments:
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).
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. 

Comments:
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.
[Suggestion] Glue Diagnostic Data with Labels 

Hello! First of all, thanks for setting up this useful project!

I've just realised you provide the the [Glue Diagnostics Data](https://huggingface.co/nlp/viewer/?dataset=glue&config=ax) without labels, indicating in the `GlueConfig` that you've only a test set.

Yet, the data with labels is available, too (see also [here](https://gluebenchmark.com/diagnostics#introduction)):

https://www.dropbox.com/s/ju7d95ifb072q9f/diagnostic-full.tsv?dl=1 

Have you considered incorporating it?

Comments:
Issues: Adding a FAISS or Elastic Search index to a Dataset

It seems the DPRContextEncoder, DPRContextEncoderTokenizer cited[ in this documentation](https://huggingface.co/nlp/faiss_and_ea.html) is not implemented ? It didnot work with the standard nlp installation . Also, I couldn't find or use it with the latest nlp install from github in Colab.  Is  there any dependency on the latest PyArrow 1.0.0 ? Is it yet to be made generally available ?

Comments:
`DPRContextEncoder` and `DPRContextEncoderTokenizer` will be available in the next release of `transformers`.

Right now you can experiment with it by installing `transformers` from the master branch.
You can also check the docs of DPR [here](https://huggingface.co/transformers/master/model_doc/dpr.html).

Moreover all the indexing features will also be available in the next release of `nlp`.

@lhoestq Thanks for the info 

@lhoestq  I tried installing transformer from the master branch. Python imports for DPR again didnt' work.  Anyways, Looking forward to trying it in the next release of nlp 

@nsankar have you tried with the latest version of the library?

@yjernite it worked. Thanks
New Datasets: IWSLT15+, ITTB

**Links:**
[iwslt](https://pytorchnlp.readthedocs.io/en/latest/_modules/torchnlp/datasets/iwslt.html)
Don't know if that link is up to date.

[ittb](http://www.cfilt.iitb.ac.in/iitb_parallel/)
**Motivation**: replicate mbart finetuning results (table below)
![image](https://user-images.githubusercontent.com/6045025/88490093-0c1c8c00-cf67-11ea-960d-8dcaad2aa8eb.png)


For future readers, we already have the following language pairs in the wmt namespaces:

```
wmt14: ['cs-en', 'de-en', 'fr-en', 'hi-en', 'ru-en']
wmt15: ['cs-en', 'de-en', 'fi-en', 'fr-en', 'ru-en']
wmt16: ['cs-en', 'de-en', 'fi-en', 'ro-en', 'ru-en', 'tr-en']
wmt17: ['cs-en', 'de-en', 'fi-en', 'lv-en', 'ru-en', 'tr-en', 'zh-en']
wmt18: ['cs-en', 'de-en', 'et-en', 'fi-en', 'kk-en', 'ru-en', 'tr-en', 'zh-en']
wmt19: ['cs-en', 'de-en', 'fi-en', 'gu-en', 'kk-en', 'lt-en', 'ru-en', 'zh-en', 'fr-de']
```

Comments:
Thanks Sam, we now have a very detailed tutorial and template on how to add a new dataset to the library. It typically take 1-2 hours to add one. Do you want to give it a try ?
The tutorial on writing a new dataset loading script is here: https://huggingface.co/nlp/add_dataset.html
And the part on how to share a new dataset is here: https://huggingface.co/nlp/share_dataset.html

Hi @sshleifer, I'm trying to add IWSLT using the link you provided but the download urls are not working. Only `[en, de]` pair is working. For others language pairs it throws a `404` error.
Google Colab - load_dataset - PyArrow exception

With latest PyArrow 1.0.0 installed, I get the following exception   . Restarting colab has the same issue

ImportWarning: To use `nlp`, the module `pyarrow>=0.16.0` is required, and the current version of `pyarrow` doesn't match this condition. If you are running this in a Google Colab, you should probably just restart the runtime to use the right version of `pyarrow`.

The error goes only when I install version 0.16.0 
i.e.  !pip install pyarrow==0.16.0

Comments:
Indeed, we’ll make a new PyPi release next week to solve this. Cc @lhoestq 

+1! this is the reason our tests are failing at [TextAttack](https://github.com/QData/TextAttack) 

(Though it's worth noting if we fixed the version number of pyarrow to 0.16.0 that would fix our problem too. But in this case we'll just wait for you all to update)

Came to raise this issue, great to see other already have and it's being fixed so soon!

As an aside, since no one wrote this already, it seems like the version check only looks at the second part of the version number making sure it is >16, but pyarrow newest version is 1.0.0 so the second past is 0!

> Indeed, we’ll make a new PyPi release next week to solve this. Cc @lhoestq

Yes definitely

please fix this on pypi! @lhoestq 

Is this issue fixed ?

We’ll release the new version later today. Apologies for the delay.

I just pushed the new version on pypi :)

Thanks for the update.
ImportWarning for pyarrow 1.0.0

The following PR raised ImportWarning at `pyarrow ==1.0.0` https://github.com/huggingface/nlp/pull/265/files

Comments:
This was fixed in #434 
We'll do a release later this week to include this fix.
Thanks for reporting

I dont know if the fix was made but the problem is still present : 
Instaled with pip : NLP 0.3.0 // pyarrow 1.0.0 
OS : archlinux with kernel zen 5.8.5

Yes it was fixed in `nlp>=0.4.0`
You can update with pip

Sorry, I didn't got the updated version, all is now working perfectly thanks
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.

Comments:
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.