helmo/github-issues huggingface dataset:
Adding IWSLT 2017  dataset.
State: closed | Author: Narsil (CONTRIBUTOR)

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
State: closed | Author: thomwolf (MEMBER)

- 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
State: closed | Author: lhoestq (MEMBER)

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 rename, remove and cast in-place operations
State: closed | Author: thomwolf (MEMBER)

Add a bunch of in-place operation leveraging the Arrow back-end to rename and remove columns and cast to new features without using the more expensive `map` method.

These methods are added to `Dataset` as well as `DatasetDict`.

Added tests for these new methods and add the methods to the doc.

Naming follows the new pattern with a trailing underscore indicating in-place methods.
Add dataset/mlsum
State: closed | Author: RachelKer (CONTRIBUTOR)

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
State: closed | Author: lhoestq (MEMBER)

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
State: closed | Author: thomwolf (MEMBER)

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`
Add bleurt
State: closed | Author: yjernite (MEMBER)

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
State: closed | Author: malikaltakrori (CONTRIBUTOR)

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
State: closed | Author: lhoestq (MEMBER)

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
State: closed | Author: mariamabarham (CONTRIBUTOR)

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
State: closed | Author: idoh (CONTRIBUTOR)

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
State: closed | Author: joshhu (NONE)
Labels: name: dataset bug

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()
State: closed | Author: vegarab (CONTRIBUTOR)

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 
State: open | Author: ggbetz (NONE)
Labels: name: Dataset discussion

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?
Add features parameter in load dataset
State: closed | Author: lhoestq (MEMBER)

Added `features` argument in `nlp.load_dataset`.
If they don't match the data type, it raises a `ValueError`.

It's a draft PR because #440 needs to be merged first.

Comments: This one is ready for review now

I changed to using features only, instead of info.
Let mw know if it sounds good to you now @thomwolf
Issues: Adding a FAISS or Elastic Search index to a Dataset
State: closed | Author: nsankar (NONE)

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
State: open | Author: sshleifer (CONTRIBUTOR)
Labels: name: dataset request

**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.