t20062443/AT1ds huggingface dataset:
patrickvonplaten (CONTRIBUTOR) commented on 2020-05-14 13:54:29:
> > 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 ;-)
astariul (NONE) commented on 2020-05-15 00:33:57:
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 ?
astariul (NONE) commented on 2020-05-15 02:41:22:
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
patrickvonplaten (CONTRIBUTOR) commented on 2020-05-15 08:56:23:
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 ...
lhoestq (MEMBER) commented on 2020-05-15 09:46:07:
For each dataset, If there exist a `dataset_info.json`, then the command `nlp-cli test path/to/my/dataset --al_configs` is successful only if the `dataset_infos.json` is correct. The infos are correct if the size and checksums of the downloaded file are correct, and if the number of examples in each split are correct.

Note: the `test` command is supposed to test the script, that's why it runs the script even if the cached files already exist. Let me know if it's good to you.
patrickvonplaten (CONTRIBUTOR) commented on 2020-05-15 10:03:14:
> For each dataset, If there exist a `dataset_info.json`, then the command `nlp-cli test path/to/my/dataset --al_configs` is successful only if the `dataset_infos.json` is correct. The infos are correct if the size and checksums of the downloaded file are correct, and if the number of examples in each split are correct.
> 
> Note: the `test` command is supposed to test the script, that's why it runs the script even if the cached files already exist. Let me know if it's good to you.

Does it have to download the whole data to check if the checksums are correct? I guess so no?
lhoestq (MEMBER) commented on 2020-05-15 10:06:11:
> > For each dataset, If there exist a `dataset_info.json`, then the command `nlp-cli test path/to/my/dataset --al_configs` is successful only if the `dataset_infos.json` is correct. The infos are correct if the size and checksums of the downloaded file are correct, and if the number of examples in each split are correct.
> > Note: the `test` command is supposed to test the script, that's why it runs the script even if the cached files already exist. Let me know if it's good to you.
> 
> Does it have to download the whole data to check if the checksums are correct? I guess so no?

Yes it has to download them all (unless they were already downloaded in which case it just uses the cached downloaded files).
zphang (NONE) commented on 2020-05-15 15:03:31:
As a follow-up to this: It looks like the actual GLUE task name is supplied as the `name` argument. Is there a way to check what `name`s/sub-datasets are available under a grouping like GLUE? That information doesn't seem to be readily available in info from `nlp.list_datasets()`.

Edit: I found the info under `Glue.BUILDER_CONFIGS`
patrickvonplaten (CONTRIBUTOR) commented on 2020-05-16 11:12:00:
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 ? :-)
pmbaumgartner (NONE) commented on 2020-05-16 12:56:47:
Chiming in to second everything @honnibal said, and to add that I think the current name is going to impact the discoverability of this library. People who are looking for "NLP Datasets" through a search engine are going to see a library called `nlp` and think it's too broad. People who are looking to do NLP in python are going to search "Python NLP" and end up here, confused that this is a collection of datasets.

The names of the other huggingface libraries work because they're the only game in town: there are not very many robust, distinct libraries for `tokenizers` or `transformers` in python, for example. But there are several options for NLP in python, and adding this as a possible search result for "python nlp" when datasets are likely not what someone is searching for adds noise and frustrates potential users.
stefan-it (CONTRIBUTOR) commented on 2020-05-16 13:51:32:
@patrickvonplaten Rebased it 😅

How can it test 🤔 I used:

```bash
RUN_SLOW=1 RUN_LOCAL=1 pytest tests/test_dataset_common.py::DatasetTest::test_load_real_dataset_local_germeval_14
# and
RUN_SLOW=1 RUN_LOCAL=1 pytest tests/test_dataset_common.py::DatasetTest::test_load_dataset_all_configs_local_germeval_14
```

and the tests still pass :)
dennlinger (NONE) commented on 2020-05-16 15:09:43:
I'm also not sure whether the naming of `nlp` is the problem itself, as long as it comes with the appropriate identifier, so maybe something like `huggingface_nlp`? This is analogous to what @honnibal and spacy are doing for `spacy-transformers`. Of course, this is a "step back" from the recent changes/renaming of transformers, but may be some middle ground between a complete rebranding, and keeping it identifiable.
richarddwang (CONTRIBUTOR) commented on 2020-05-17 01:45:08:
As far as I understand, `wikitext` is refer to `WikiText-103` and `WikiText-2` that created by researchers in Salesforce, and mostly used in traditional language modeling.

You might want to say `wikipedia`, a dump from wikimedia foundation.

Also I would like to have Toronto BookCorpus too ! Though it involves copyright problem...
thomwolf (MEMBER) commented on 2020-05-17 12:54:28:
Hi @zphang,

So you can just give the local path to a dataset script file and it should work.

Here is an example:
- you can download one of the scripts in the `datasets` folder of the present repo (or clone the repo)
- then you can load it with `load_dataset('PATH/TO/YOUR/LOCAL/SCRIPT.py')`

Does it make sense?
thomwolf (MEMBER) commented on 2020-05-17 13:01:01:
Yes so the first config is loaded by default when no `name` is supplied but for GLUE this should probably throw an error indeed.

We can probably just add an `__init__` at the top of the `class Glue(nlp.GeneratorBasedBuilder)` in the `glue.py` script which does this check:
```
class Glue(nlp.GeneratorBasedBuilder):
    def __init__(self, *args, **kwargs):
        assert 'name' in kwargs and kwargs[name] is not None, "Glue has to be called with a configuration name"
        super(Glue, self).__init__(*args, **kwargs)
```
thomwolf (MEMBER) commented on 2020-05-17 13:10:12:
Interesting, thanks for sharing your thoughts.

As we’ll move toward a first non-beta release, we will pool the community of contributors/users of the library for their opinions on a good final name (like when we renamed the beautifully (?) named `pytorch-pretrained-bert`)

In the meantime, using `from nlp import load_dataset, load_metric` should work 😉
astariul (NONE) commented on 2020-05-17 22:21:47:
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
thomwolf (MEMBER) commented on 2020-05-18 07:52:52:
Actually, double checking with @mariamabarham, we already have this feature I think.

It's like this currently:
```python
>>> from nlp import load_dataset
>>> 
>>> dataset = load_dataset('glue', 'cola', split='train')
>>> print(dataset.info.citation)
@article{warstadt2018neural,
  title={Neural Network Acceptability Judgments},
  author={Warstadt, Alex and Singh, Amanpreet and Bowman, Samuel R},
  journal={arXiv preprint arXiv:1805.12471},
  year={2018}
}
@inproceedings{wang2019glue,
  title={{GLUE}: A Multi-Task Benchmark and Analysis Platform for Natural Language Understanding},
  author={Wang, Alex and Singh, Amanpreet and Michael, Julian and Hill, Felix and Levy, Omer and Bowman, Samuel R.},
  note={In the Proceedings of ICLR.},
  year={2019}
}

Note that each GLUE dataset has its own citation. Please see the source to see
the correct citation for each contained dataset.
```

What do you think @dseddah?
mariamabarham (CONTRIBUTOR) commented on 2020-05-18 10:17:55:
@AlphaMycelium the Ubuntu Dialogue Corpus [version 2]( https://github.com/rkadlec/ubuntu-ranking-dataset-creator) is added. Note that it requires a manual download by following the download instructions in the [repos]( https://github.com/rkadlec/ubuntu-ranking-dataset-creator).
Maybe we can close this issue for now?
julien-c (MEMBER) commented on 2020-05-18 16:45:15:
I feel like we are conflating two distinct subjects here:

1. @honnibal's point is that using `nlp` as a package name might break existing code and bring developer usability issues in the future
2. @pmbaumgartner's point is that the `nlp` package name is too broad and shouldn't be used by a package that exposes only datasets and metrics

(let me know if I mischaracterize your point)

I'll chime in to say that the first point is a bit silly IMO. As Python developers due to the limitations of the import system we already have to share:
- a single flat namespace for packages
- which also conflicts with local modules i.e. local files

If we add the constraint that this flat namespace also be shared with variable names this gets untractable pretty fast :)

I also think all Python software developers/ML engineers/scientists are capable of at least a subset of:
- importing only the methods that they need like @thomwolf suggested
- aliasing their import
- renaming a local variable
dpressel (NONE) commented on 2020-05-18 21:23:15:
Hi, the full example was listed in the PR above, but here is the exact link:

https://github.com/dpressel/mead-baseline/blob/3c1aa3ca062cb23f303ca98ac40b6652b37ee971/api-examples/layers-classify-hf-datasets.py

The problem is coming from
```
            if cache_file_name is None:
                # we create a unique hash from the function, current dataset file and the mapping args
                cache_kwargs = {
                    "with_indices": with_indices,
                    "batched": batched,
                    "batch_size": batch_size,
                    "remove_columns": remove_columns,
                    "keep_in_memory": keep_in_memory,
                    "load_from_cache_file": load_from_cache_file,
                    "cache_file_name": cache_file_name,
                    "writer_batch_size": writer_batch_size,
                    "arrow_schema": arrow_schema,
                    "disable_nullable": disable_nullable,
                }
                cache_file_name = self._get_cache_file_path(function, cache_kwargs)
```
The cached value is always the same, but I was able to change that by just renaming the function each time which seems to fix the issue.
EntilZha (CONTRIBUTOR) commented on 2020-05-18 21:34:01:
I have an initial version here: https://github.com/EntilZha/nlp/tree/master/datasets/qanta Thats pretty close to what I'll do as a PR, but still want to do some more sanity checks/tests (just got tests passing).

I figured out how to get all tests passing by adding a download command and some finagling with the data zip https://github.com/EntilZha/nlp/blob/master/tests/utils.py#L127
patrickvonplaten (CONTRIBUTOR) commented on 2020-05-18 21:56:24:
I'm quite positive that you can just replace the `dl_manager.download()` statements here: https://github.com/EntilZha/nlp/blob/4d46443b65f1f756921db8275594e6af008a1de7/datasets/qanta/qanta.py#L194 with `dl_manager.download_and_extract()` even though you don't extract anything. I would prefer to avoid adding more functions to the MockDataLoadManager and keep it as simple as possible (It's already to complex now IMO). 

Could you check if you can replace the `download()` function?
EntilZha (CONTRIBUTOR) commented on 2020-05-18 23:49:51:
I might be doing something wrong, but swapping those two gives this error:
```
>       with open(path) as f:
E       IsADirectoryError: [Errno 21] Is a directory: 'datasets/qanta/dummy/mode=first,char_skip=25/2018.4.18/dummy_data-zip-extracted/dummy_data'

src/nlp/datasets/qanta/3d965403133687b819905ead4b69af7bcee365865279b2f797c79f809b4490c3/qanta.py:280: IsADirectoryError

During handling of the above exception, another exception occurred:
```

So it seems like the directory name is getting passed. Is this not functioning as expected, or is there some caching happening maybe? I deleted the dummy files and re-ran the import script with no changes. I'm digging a bit in with a debugger, but no clear reason yet
EntilZha (CONTRIBUTOR) commented on 2020-05-18 23:56:58:
From what I can tell here: https://github.com/huggingface/nlp/blob/master/tests/utils.py#L115

1. `data_url` is the correct http link
2. `path_to_dummy_data` is a directory, which is causing the issue

That path comes from `download_dummy_data`, which I think assumes that the data comes from the zip file, but isn't aware of individual files. So it seems like it data manager needs to be aware if the url its getting is for a file or a zip/directory, and pass this information along. This might happen in `download_dummy_data`, but probably better to happen in `download_and_extract`? Maybe a simple check to see if `os.path.basename` returns the dummy data zip filename, if not then join paths with the basename of the url?
patrickvonplaten (CONTRIBUTOR) commented on 2020-05-19 09:04:40:
I think the dataset script works correctly. Just the dummy data structure seems to be wrong. I will soon add more commands that should make the create of the dummy data easier.

I'd recommend that you won't concentrate too much on the dummy data.
If you manage to load the dataset correctly via:

```python 
# use local path to qanta
nlp.load_dataset("./datasets/qanta")
```

then feel free to open a PR and we will look into the dummy data problem together :-) 

Also please make sure that the Version is in the format 1.0.0 (three numbers separated by two points) - not a date.
tomhosking (NONE) commented on 2020-05-19 09:26:29:
Hi @patrickvonplaten!

I'm now getting the below error:

```
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-28-3206959998b9> in <module>
      1 import nlp
      2 
----> 3 dataset = nlp.load_dataset('wmt14')
      4 print(dataset['train'][0])

~/.local/lib/python3.6/site-packages/nlp/load.py in load_dataset(path, name, version, data_dir, data_files, split, cache_dir, download_config, download_mode, ignore_verifications, save_infos, **config_kwargs)
    507     # Instantiate the dataset builder
    508     builder_instance = builder_cls(
--> 509         cache_dir=cache_dir, name=name, version=version, data_dir=data_dir, data_files=data_files, **config_kwargs,
    510     )
    511 

TypeError: Can't instantiate abstract class Wmt with abstract methods _subsets
```
thomwolf (MEMBER) commented on 2020-05-19 12:47:17:
By the way, `nlp` will very likely not be only about datasets, and not even just about datasets and metrics.

I see it as a laboratory for testing several long-term ideas about how we could do NLP in terms of research as well as open-source and community sharing, most of these ideas being too experimental/big to fit in `transformers`.

Some of the directions we would like to explore are about sharing, traceability and more experimental models, as well as seeing a model as the community-based process of creating a composite entity from data, optimization, and code.

We'll see how these ideas end up being implemented and we'll better know how we should define the library when we start to dive into these topics. I'll try to get the `nlp` team to draft a roadmap on these topics at some point.
patrickvonplaten (CONTRIBUTOR) commented on 2020-05-19 12:49:33:
To correct this error I think you need the master branch of `nlp`. Can you try to install `nlp` with. `WMT` was not included at the beta release of the library. 

Can you try:
`pip install git+https://github.com/huggingface/nlp.git`

and check again?
patrickvonplaten (CONTRIBUTOR) commented on 2020-05-19 16:18:49:
> The script loading seems to work fine so I'll work on getting a PR open after a few sanity checks on the data.
> 
> On version, we currently have it versioned with YYYY.MM.DD scheme so it would be nice to not change that, but will it cause issues?

It would cause issues for sure for the tests....not sure if it would also cause issues otherwise.

I would prefer to keep the same version style as we have for other models. You could for example simply add version 1.0.0 and add a comment with the date you currently use for the versioning.

 What is your opinion regarding the version here @lhoestq @mariamabarham @thomwolf ?
mariamabarham (CONTRIBUTOR) commented on 2020-05-19 17:00:28:
> Maybe use the YYYY.MM.DD as the config name ? That's what we are doing for wikipedia

I'm not sure if this will work because the name should be unique and it seems that he has multiple config name in his data with the same version.
As @patrickvonplaten  suggested, I think  you can add a comment about the version in the data description.
thomwolf (MEMBER) commented on 2020-05-19 21:45:42:
This is kinda related to one thing I had in mind which is that we may want to be able to dump our model predictions in a `Dataset` as well so that we don't keep them in memory (and we can export them in a nice format later as well when we will have a serialization formats).

Maybe this is overkill though, I haven't fully wraped my head around this.
EntilZha (CONTRIBUTOR) commented on 2020-05-19 23:11:04:
I'm good with either putting it in description, adding it to the config, or loosening version formatting. I mostly don't have a full conceptual grasp of what each identifier ends up meaning in the datasets code so hard to evaluate the best approach.

For background, the multiple formats is a consequence of:

1. Each example is one multi-sentence trivia question
2. For training, its better to treat each sentence as an example
3. For evaluation, should test on: (1) first sentence, (2) full question, and (3) partial questions (does the model get the question right having seen the first half)

We use the date format for version since: (1) we expect some degree of updates since new questions come in every year and (2) the timestamp itself matches the Wikipedia dump that it is dependent on (so similar to the Wikipedia dataset).

perhaps this is better discussed in https://github.com/huggingface/nlp/pull/169 or update title?
patrickvonplaten (CONTRIBUTOR) commented on 2020-05-20 08:53:21:
Usually, the idea is that a dataset can be added without releasing a new version. The problem in the case of `WMT` was that some "core" code of the library had to be changed as well. 

@thomwolf @lhoestq @julien-c - How should we go about this. If we add a dataset that also requires "core" code changes, how do we handle the versioning? The moment a dataset is on AWS it will actually be listed with `list_datasets()` in all earlier versions...

Is there a way to somehow insert the `pip version` to the HfApi() and get only the datasets that were available for this version (at the date of the release of the version) @julien-c ?
lhoestq (MEMBER) commented on 2020-05-20 15:23:34:
This is the format needed to run the official script directly. The format of the squad dataset is different from the input of the metric. 

> One thing for SQuAD is that I wanted to be able to use the SQuAD dataset directly in the metrics and I'm not sure it will be possible with this format.
> 
> (maybe it's not really possible in general though)

Ok I see. I'll try to use the same format
lhoestq (MEMBER) commented on 2020-05-20 15:36:58:
Ok with this update I changed the format to fit the squad dataset format.
Now you can do:
```python
squad_dset = nlp.load_dataset("squad")
squad_metric = nlp.load_metric("/Users/quentinlhoest/Desktop/hf/nlp-bis/metrics/squad")
predictions = [
    {"id": v["id"], "prediction_text": v["answers"]["text"][0]}  # take first possible answer
    for v in squad_dset["validation"]
]
squad_metric.compute(predictions, squad_dset["validation"])
```
korakot (NONE) commented on 2020-05-21 17:23:20:
I now try with the sample `datasets/csv` folder. 

    nlp-cli upload csv

The error is still the same

```
2020-05-21 17:20:56.394659: I tensorflow/stream_executor/platform/default/dso_loader.cc:44] Successfully opened dynamic library libcudart.so.10.1
About to upload file /content/csv/csv.py to S3 under filename csv/csv.py and namespace korakot
About to upload file /content/csv/dummy/0.0.0/dummy_data.zip to S3 under filename csv/dummy/0.0.0/dummy_data.zip and namespace korakot
Proceed? [Y/n] y
Uploading... This might take a while if files are large
Traceback (most recent call last):
  File "/usr/local/bin/nlp-cli", line 33, in <module>
    service.run()
  File "/usr/local/lib/python3.6/dist-packages/nlp/commands/user.py", line 234, in run
    token=token, filename=filename, filepath=filepath, organization=self.args.organization
  File "/usr/local/lib/python3.6/dist-packages/nlp/hf_api.py", line 141, in presign_and_upload
    urls = self.presign(token, filename=filename, organization=organization)
  File "/usr/local/lib/python3.6/dist-packages/nlp/hf_api.py", line 132, in presign
    return PresignedUrl(**d)
TypeError: __init__() got an unexpected keyword argument 'cdn'
```
MaveriQ (NONE) commented on 2020-05-21 19:40:31:
Could you give a more concrete example, please? 

I looked up wikitext dataset script from the repo. Should I just overwrite the `data_file` on line 98 to point to the local dataset directory? Would it work for different configurations of wikitext (wikitext2, wikitext103 etc.)?

Or maybe we can use DownloadManager to specify local dataset location? In that case, where do we use DownloadManager instance?

Thanks