t20062443/AT1ds huggingface dataset:
patrickvonplaten (CONTRIBUTOR) commented on 2020-07-13 13:59:52:
@mariamabarham This script should work. I tested it for a couple of datasets. There might be exceptions where the script breaks - did not test everything.

```python
#!/usr/bin/env python3

from nlp import prepare_module, DownloadConfig, import_main_class, hf_api
import tempfile


def scan_for_nested_unnecessary_dict(dataset_name):

    def load_builder_class(dataset_name):
        module_path = prepare_module(dataset_name, download_config=DownloadConfig(force_download=True))
        return import_main_class(module_path)

    def load_configs(dataset_name):
        builder_cls = load_builder_class(dataset_name)
        if len(builder_cls.BUILDER_CONFIGS) == 0:
            return [None]
        return builder_cls.BUILDER_CONFIGS

    def scan_features_for_nested_dict(features):
        is_sequence = False
        if hasattr(features, "_type"):
            if features._type != 'Sequence':
                return False
            else:
                is_sequence = True
                features = features.feature

        if isinstance(features, list):
            for value in features:
                if scan_features_for_nested_dict(value):
                    return True
            return False

        elif isinstance(features, dict):
            for key, value in features.items():
                if is_sequence and len(features.keys()) == 1 and hasattr(features[key], "_type") and features[key]._type != "Sequence":
                    return True
                if scan_features_for_nested_dict(value):
                    return True
            return False
        elif hasattr(features, "_type"):
            return False
        else:
            raise ValueError(f"{features} should be either a list, a dict or a feature")

    configs = load_configs(dataset_name)

    for config in configs:
        with tempfile.TemporaryDirectory() as processed_temp_dir:
            # create config and dataset
            dataset_builder_cls = load_builder_class(dataset_name)
            name = config.name if config is not None else None
            dataset_builder = dataset_builder_cls(name=name, cache_dir=processed_temp_dir)

        is_nested_dict_in_dataset = scan_features_for_nested_dict(dataset_builder._info().features)
        if is_nested_dict_in_dataset:
            print(f"{dataset_name} with {name} needs refactoring")


if __name__ == "__main__":
    scan_for_nested_unnecessary_dict("race")  # prints True
    scan_for_nested_unnecessary_dict("mlqa")  # prints True
    scan_for_nested_unnecessary_dict("squad")  # prints Nothing

    # ran the following lines for 1min and seems to work -> didn't check for all datasets though
#    api = hf_api.HfApi()
#    all_datasets = [x.id for x in api.dataset_list(with_community_datasets=False)]
#    for dataset in all_datasets:
#        scan_for_nested_unnecessary_dict(dataset)
```
mariamabarham (CONTRIBUTOR) commented on 2020-07-13 14:43:38:
> @mariamabarham This script should work. I tested it for a couple of datasets. There might be exceptions where the script breaks - did not test everything.
> 
> ```python
> #!/usr/bin/env python3
> 
> from nlp import prepare_module, DownloadConfig, import_main_class, hf_api
> import tempfile
> 
> 
> def scan_for_nested_unnecessary_dict(dataset_name):
> 
>     def load_builder_class(dataset_name):
>         module_path = prepare_module(dataset_name, download_config=DownloadConfig(force_download=True))
>         return import_main_class(module_path)
> 
>     def load_configs(dataset_name):
>         builder_cls = load_builder_class(dataset_name)
>         if len(builder_cls.BUILDER_CONFIGS) == 0:
>             return [None]
>         return builder_cls.BUILDER_CONFIGS
> 
>     def scan_features_for_nested_dict(features):
>         is_sequence = False
>         if hasattr(features, "_type"):
>             if features._type != 'Sequence':
>                 return False
>             else:
>                 is_sequence = True
>                 features = features.feature
> 
>         if isinstance(features, list):
>             for value in features:
>                 if scan_features_for_nested_dict(value):
>                     return True
>             return False
> 
>         elif isinstance(features, dict):
>             for key, value in features.items():
>                 if is_sequence and len(features.keys()) == 1 and hasattr(features[key], "_type") and features[key]._type != "Sequence":
>                     return True
>                 if scan_features_for_nested_dict(value):
>                     return True
>             return False
>         else:
>             raise ValueError(f"{features} should be either a list of a dict")
> 
>     configs = load_configs(dataset_name)
> 
>     for config in configs:
>         with tempfile.TemporaryDirectory() as processed_temp_dir:
>             # create config and dataset
>             dataset_builder_cls = load_builder_class(dataset_name)
>             name = config.name if config is not None else None
>             dataset_builder = dataset_builder_cls(name=name, cache_dir=processed_temp_dir)
> 
>         is_nested_dict_in_dataset = scan_features_for_nested_dict(dataset_builder._info().features)
>         if is_nested_dict_in_dataset:
>             print(f"{dataset_name} with {name} needs refactoring")
> 
> 
> if __name__ == "__main__":
>     scan_for_nested_unnecessary_dict("race")  # prints True
>     scan_for_nested_unnecessary_dict("mlqa")  # prints True
>     scan_for_nested_unnecessary_dict("squad")  # prints Nothing
> 
>     # ran the following lines for 1min and seems to work -> didn't check for all datasets though
> #    api = hf_api.HfApi()
> #    all_datasets = [x.id for x in api.dataset_list(with_community_datasets=False)]
> #    for dataset in all_datasets:
> #        scan_for_nested_unnecessary_dict(dataset)
> ```

Great, I will try it
mitchellgordon95 (CONTRIBUTOR) commented on 2020-07-14 21:56:27:
By the way, the reason this is an issue for me is because I want to be able to "save" changes made to a dataset by writing something to disk. In this case, I would like to pre-process my dataset once, and then train multiple models on the dataset later without having to re-process the data. 

Is pickling/unpickling the Dataset object the "sanctioned" way of doing this? Or is there a better way that I'm missing?
jarednielsen (CONTRIBUTOR) commented on 2020-07-14 23:36:20:
I've had success with saving datasets to disk via:

```python
cache_file = "/my/dset.cache"
dset = dset.map(whatever, cache_file_name=cache_file)
# then, later
dset = nlp.Dataset.from_file(cache_file)
```

This restores the dataset with all the attributes I need.
mitchellgordon95 (CONTRIBUTOR) commented on 2020-07-15 00:23:54:
Thanks @jarednielsen, that makes sense. I'm a little wary of messing with the cache files, since I still don't really understand what's going on under the hood with Apache Arrow. 

Related question: I'd like to do parallel pre-processing of the dataset. I know how to break the dataset up via sharding, but is there any way to combine the shards back together again once the processing is done? Right now I'm probably just going to iterate over each shard, write the contexts to a txt file, and then cat the txt files, but it feels like there ought to be a nicer way to concatenate datasets.
ericaliu0610 (NONE) commented on 2020-07-15 11:31:38:
similar slow download speed here for nlp.load_dataset('wmt14', 'fr-en')
`
Downloading: 100%|██████████████████████████████████████████████████████████| 658M/658M [1:00:42<00:00, 181kB/s]
Downloading: 100%|██████████████████████████████████████████████████████████| 918M/918M [1:39:38<00:00, 154kB/s]
Downloading:   2%|▉                                                       | 40.9M/2.37G [04:48<5:03:06, 128kB/s]
`
Could we just download a specific subdataset in 'wmt14', such as 'newstest14'?
lhoestq (MEMBER) commented on 2020-07-15 11:33:12:
Looks cool :)

I feel like 
```python
concatenated_dataset = dataset1.concatenate(dataset2)
```
could be more natural. What do you think ?

Also could you also concatenate the `nlp.Dataset._data_files` ?
```python
return cls(table, info=info, split=split, data_files=self._data_files + other_dataset._data_files)
```
ghomasHudson (CONTRIBUTOR) commented on 2020-07-15 11:59:19:
@lhoestq do you think it's acceptable for the `dl_manager` to go grab all the individual stories from project gutenburg? I've got a working version of that but it does clutter up your huggingface cache somewhat.

The real value (and original purpose) of this dataset is doing question answering on the full text.
lhoestq (MEMBER) commented on 2020-07-15 12:08:27:
> @lhoestq do you think it's acceptable for the `dl_manager` to go grab all the individual stories from project gutenburg? I've got a working version of that but it does clutter up your huggingface cache somewhat.
> 
> The real value (and original purpose) of this dataset is doing question answering on the full text.

What's the problem exactly with the cache ?
gaguilar (CONTRIBUTOR) commented on 2020-07-15 14:18:51:
Awesome! Thank you for all your comments! 👌  I will update the PR in a bit with all the required changes 🙂 

Let me just provide a bit of context for my changes:

I was referring to the GLUE, XTREME and WNUT_17 dataset scripts to build mine (not sure if the new documentation was available last week). This is where I took the naming convention for the citation and description variables. Also, these scripts didn't have the `BUILDER_CONFIG_CLASS = LinceConfig` line so I commented this out thinking I didn't need that; I tried this line in my attempts to make the real and dummy data tests pass but it was not helping. 

The problem I was facing was that the tests were passing a default `BuilderConfig` (i.e., `self.config.name` property was set to `'default'` and my custom properties were not available). This means, for example, that within the `def _info(...)`  method, I was not able to access the specific fields of my `LinceConfig` class (which is why I have now a global variable `_LINCE_CITATIONS`, to detach the individual citations from the corresponding LinceConfig objects, as well as I am constructing manually the feature infos). This default `BuilderConfig` is why I added the `if not isinstance(self.config, LinceConfig): return []` statement. Otherwise, accessing custom properties like `self.config.colnames` was failing the test because such properties did not exist in the default config (i.e., it was not a `LinceConfig`).

I will update the PR and see if these problems happen in the CI tests.

Thanks again for the follow-up! @lhoestq
lhoestq (MEMBER) commented on 2020-07-15 14:29:07:
Ok I see !

To give you more details: the line `BUILDER_CONFIG_CLASS = LinceConfig` tells the tests how to instantiate a config for this dataset. Therefore if you have this line you should have all the fields of your config available.

To fix the errors you get you'll have to, first, have the `BUILDER_CONFIG_CLASS = LinceConfig` line, and second, add default values for the parameters of your config (or the tests functions will be unable to instantiate it by calling `LinceConfig()`.

An example of dataset with a custom config with additional filed like this one is [biomrc](https://github.com/huggingface/nlp/blob/master/datasets/biomrc/biomrc.py).
Feel free to give a look at it if you want.
mitchellgordon95 (CONTRIBUTOR) commented on 2020-07-15 15:00:12:
Ah, yes, you are correct. The pickle file contains the whole dataset, not just the cache names, which is not quite what I expected.

I tried adding a warning when pickling a Dataset, to prevent others like me from trying it. Interestingly, however, the warning is raised whenever any function on the dataset is called (select, shard, etc.). 

```
import nlp
wiki = nlp.load_dataset('wikipedia', split='train')
wiki = wiki.shard(16, 0) # Triggers pickling of dataset
```

I believe this is because [this line](https://github.com/huggingface/nlp/blob/master/src/nlp/arrow_dataset.py#L626), which gets the function signature, is actually pickling the whole dataset (and thereby serializing all the data to text). I checked by printing that string, and sure enough it was full of Wikipedia articles.

I don't think the whole pickling thing is worth the effort, so I'll close the PR. But I did want to mention this serialization behavior in case it's not intended.
jarednielsen (CONTRIBUTOR) commented on 2020-07-15 15:57:19:
Thanks for the suggestion! `dset1.concatenate(dset2)` does feel more natural. Although this seems to be a different "class" of transformation function than map() or filter(), acting on two datasets rather than on one. I would prefer the function signature treat both datasets symmetrically.

Python lists have `list1 + list2` or `list1.extend(list2)`.
NumPy has `np.concatenate((arr1, arr2))`.
Pandas has `pd.join((df1, df2))`.
PyTorch has `ConcatDataset((dset1, dset2))`.

Given the symmetrical treatment and clear communication that this creates a new object, rather than a simple chaining on the first, my preference is now for `nlp.concatenate((dset1, dset2))`. This would place the function in the same API class as `nlp.load_dataset`. Does that work?
jarednielsen (CONTRIBUTOR) commented on 2020-07-15 16:02:32:
The multi-task discussion is interesting, thanks for pointing me to that! I'll be focusing on T5 in a few weeks, so I'm sure I'll have many opinions then :). For now, I think a simple concatenate feature is important and orthogonal to that discussion. For example, a user may want to create a custom dataset that joins Wikipedia with their own custom text.
lhoestq (MEMBER) commented on 2020-07-15 16:31:19:
> Given the symmetrical treatment and clear communication that this creates a new object, rather than a simple chaining on the first, my preference is now for `nlp.concatenate((dset1, dset2))`. This would place the function in the same API class as `nlp.load_dataset`. Does that work?

Yep I like this idea. Maybe `nlp.concatenate_datasets()` ?

> For now, I think a simple concatenate feature is important and orthogonal to that discussion. For example, a user may want to create a custom dataset that joins Wikipedia with their own custom text.

I agree :)
eltoto1219 (CONTRIBUTOR) commented on 2020-07-16 01:33:51:
Okay awesome! I just added your suggestions and changed up my recursive functions. 

Here is the traceback for the when I use the original code in the write_on_file method:

```
Traceback (most recent call last):
  File "<stdin>", line 33, in <module>
  File "/home/eltoto/nlp/src/nlp/arrow_writer.py", line 214, in finalize
    self.write_on_file()
  File "/home/eltoto/nlp/src/nlp/arrow_writer.py", line 134, in write_on_file
    pa_array = pa.array(self.current_rows, type=self._type)
  File "pyarrow/array.pxi", line 269, in pyarrow.lib.array
  File "pyarrow/array.pxi", line 38, in pyarrow.lib._sequence_to_array
  File "pyarrow/error.pxi", line 106, in pyarrow.lib.check_status
pyarrow.lib.ArrowNotImplementedError: MakeBuilder: cannot construct builder for type extension<arrow.py_extension_type>

shell returned 1
```

I think when trying to cast an extension array within a list of dictionaries, some method gets called that bugs out Arrow and somehow doesn't get called when adding a single row to a a table and then appending multiple tables together. I tinkered with this for a while but could not find any workaround. 

In the case that this new method causes bad compression/worse performance, we can explicitly set the batch size in the pa.Table.to_batches(***batch_size***) method, which will return a list of batches. Perhaps, we can check that the batch size is not too large converting the table to batches after X many rows are appended to it by following the batch_size check below.
lhoestq (MEMBER) commented on 2020-07-16 08:53:50:
To convert from arrow type we have three options: to_numpy, to_pandas and to_pydict/to_pylist.

- to_numpy and to_pandas return numpy arrays instead of lists but are very fast.
- to_pydict/to_pylist can be 100x slower and become the bottleneck for reading data, but at least they return lists.

Maybe we can have to_pydict/to_pylist as the default and use to_numpy or to_pandas when the format (set by `set_format`) is 'numpy' or 'pandas'
lhoestq (MEMBER) commented on 2020-07-16 12:14:01:
One final thing: now that we have a brand new documentation, could you just add `export` to the list of documented methods in [docs/source/package_reference/main_classes.rst](https://github.com/huggingface/nlp/blob/master/docs/source/package_reference/main_classes.rst) (so that it will appear in the docs [here](https://huggingface.co/nlp/package_reference/main_classes.html)) ?
jplu (CONTRIBUTOR) commented on 2020-07-16 12:47:18:
Hey @jarednielsen 

Thanks for this very interesting analysis!! IMHO to read text data one should use `tf.data.TextLineDataset`. It would be interesting to compare what you have done with simply load with a `TextLineDataset` and see if there is a difference.

A good example can be found here https://www.tensorflow.org/tutorials/load_data/text
lhoestq (MEMBER) commented on 2020-07-16 13:45:22:
I am not able to reproduce this issue on my side.
Could you give us more details about the inputs you used ?

I do get another error though:
```
~/.virtualenvs/hf-datasets/lib/python3.7/site-packages/bert_score/utils.py in bert_cos_score_idf(model, refs, hyps, tokenizer, idf_dict, verbose, batch_size, device, all_layers)
    371         return sorted(list(set(l)), key=lambda x: len(x.split(" ")))
    372 
--> 373     sentences = dedup_and_sort(refs + hyps)
    374     embs = []
    375     iter_range = range(0, len(sentences), batch_size)

ValueError: operands could not be broadcast together with shapes (0,) (2,)
```
That's because it gets numpy arrays as input and not lists. See #387
lhoestq (MEMBER) commented on 2020-07-16 16:09:25:
Could you try to update pyarrow to >=0.17.0 @vegarab ?
I don't have any segmentation fault with my version of pyarrow (0.17.1)

I tested with
```python
import nlp
s = nlp.load_dataset("json", data_files="train-v2.0.json", field="data", split="train")
s[0]
# {'title': 'Normans', 'paragraphs': [{'qas': [{'question': 'In what country is Normandy located?', 'id':...
```
jarednielsen (CONTRIBUTOR) commented on 2020-07-17 00:25:05:
Thanks! I'm not actually loading in raw text data, that was just the synthetic data I created for this benchmark. A more realistic use case would be a dataset of tokenized examples, which would be a dict of lists of integers. TensorFlow's TextLineDataset greedily loads the dataset into the graph itself, which can lead to out-of-memory errors - one of the main reason I'm so drawn to the `nlp` library is its zero-copy no-RAM approach to dataset loading and mapping. 

It's quite helpful for running a preprocessing pipeline - a sample ELECTRA pipeline I've built is here: https://github.com/jarednielsen/deep-learning-models/blob/nlp/models/nlp/common/preprocess.py.
cosmeowpawlitan (CONTRIBUTOR) commented on 2020-07-17 04:31:52:
@lhoestq 
The error message is same as before:
Exception has occurred: UnicodeDecodeError
'cp950' codec can't decode byte 0xe2 in position 111: illegal multibyte sequence
  File "D:\python\test\test.py", line 3, in <module>
    dataset = load_dataset('xtreme', 'tydiqa')

![image](https://user-images.githubusercontent.com/50871412/87748794-7c216880-c829-11ea-94f0-7caeacb4d865.png)

I said that I found 4 copy of xtreme.py and add the 「, encoding='utf-8'」 parameter to the open() function
these python script was found under this directory
C:\Users\USER\AppData\Local\Programs\Python\Python37\Lib\site-packages\nlp\datasets\xtreme
jplu (CONTRIBUTOR) commented on 2020-07-17 07:41:11:
Sorry, I think I badly expressed myself, my bad. What I suggested is to compare with the usual loading textual data in pure TF with `TextLineDataset` with `nlp`. I know it is not recommended with very large datasets to use it, but I was curious to see how it behaves compared to a processing with `nlp` on smaller datasets.

BTW your script looks very interesting, thanks for sharing!!
ghomasHudson (CONTRIBUTOR) commented on 2020-07-17 08:50:15:
Looks good @lhoestq . The problem I'm having at the moment is that stories from project Gutenberg occasionally fail. All books are out of copyright so we should be able to host them. 

Here's a zip file of the full text if we have anywhere to put them: https://drive.google.com/file/d/17jOR7NqvzDwSlPXrlHaYV-PGI8JG-KY5/view?usp=sharing
lhoestq (MEMBER) commented on 2020-07-17 11:35:24:
> The code runs but the download speed is extremely slow, the same behaviour is not observed on wmt16 and wmt18

The original source for the files may provide slow download speeds.
We can probably host these files ourselves.

> When trying to download wmt17 zh-en, I got the following error:
> ConnectionError: Couldn't reach https://storage.googleapis.com/tfdataset-data/downloadataset/uncorpus/UNv1.0.en-zh.tar.gz

Looks like the file`UNv1.0.en-zh.tar.gz` is missing, or the url changed. We need to fix that

> Could we just download a specific subdataset in 'wmt14', such as 'newstest14'?

Right now I don't think it's possible. Maybe @patrickvonplaten knows more about it
lhoestq (MEMBER) commented on 2020-07-17 13:52:17:
> @lhoestq for all the `select`-based methods I think we should stay in Arrow format and update the writer so that it can accept Arrow tables or batches as well. What do you think?

I just tried with `writer.write_table` with tables of 1000 elements and it's slower that the solution in #405 

On my side (select 10 000 examples):
- Original implementation: 12s
- Batched solution: 100ms
- solution using arrow tables: 350ms

I'll try with arrays and record batches to see if we can make it work.
lhoestq (MEMBER) commented on 2020-07-20 09:38:47:
`from_dict` was added in #350 that was unfortunately not included in the 0.3.0 release. It's going to be included in the next release that will be out pretty soon though.
Right now if you want to use `from_dict` you have to install the package from the master branch
```
pip install git+https://github.com/huggingface/nlp.git
```
lhoestq (MEMBER) commented on 2020-07-20 10:18:16:
Now if you re-run the notebook, the two printed results are the same @colanim
```
['0.3356', '0.1466', '0.2318']
['0.3356', '0.1466', '0.2318']
```
However across sessions, the results may change (as numpy's random seed can be different). You can prevent that by setting your seed:
```python
rouge = nlp.load_metric('rouge', seed=42)
```