Repository: huggingface/datasets
Pull request: Better message when data files is empty
Fix #581 

Problem statement:
Better error message when input file does not exist
In the following scenario, when `data_files` is an empty list, the stack trace and error message could be improved. This can probably be solved by checking for each file whether it actually exists and/or whether the argument is not false-y.

```python
dataset = load_dataset("text", data_files=[])
```

Example error trace.

```
Using custom data configuration default
Downloading and preparing dataset text/default-d18f9b6611eb8e16 (download: Unknown size, generated: Unknown size, post-processed: Unknown sizetotal: Unknown size) to C:\Users\bramv\.cache\huggingface\datasets\text\default-d18f9b6611eb8e16\0.0.0\3a79870d85f1982d6a2af884fde86a71c771747b4b161fd302d28ad22adf985b...
Traceback (most recent call last):
  File "C:\Users\bramv\.virtualenvs\dutch-simplification-nbNdqK9u\lib\site-packages\nlp\builder.py", line 424, in incomplete_dir
    yield tmp_dir
  File "C:\Users\bramv\.virtualenvs\dutch-simplification-nbNdqK9u\lib\site-packages\nlp\builder.py", line 462, in download_and_prepare
    self._download_and_prepare(
  File "C:\Users\bramv\.virtualenvs\dutch-simplification-nbNdqK9u\lib\site-packages\nlp\builder.py", line 537, in _download_and_prepare
    self._prepare_split(split_generator, **prepare_split_kwargs)
  File "C:\Users\bramv\.virtualenvs\dutch-simplification-nbNdqK9u\lib\site-packages\nlp\builder.py", line 813, in _prepare_split
    num_examples, num_bytes = writer.finalize()
  File "C:\Users\bramv\.virtualenvs\dutch-simplification-nbNdqK9u\lib\site-packages\nlp\arrow_writer.py", line 217, in finalize
    self.pa_writer.close()
AttributeError: 'NoneType' object has no attribute 'close'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:/dev/python/dutch-simplification/main.py", line 7, in <module>
    dataset = load_dataset("text", data_files=files)
  File "C:\Users\bramv\.virtualenvs\dutch-simplification-nbNdqK9u\lib\site-packages\nlp\load.py", line 548, in load_dataset
    builder_instance.download_and_prepare(
  File "C:\Users\bramv\.virtualenvs\dutch-simplification-nbNdqK9u\lib\site-packages\nlp\builder.py", line 470, in download_and_prepare
    self._save_info()
  File "c:\users\bramv\appdata\local\programs\python\python38\lib\contextlib.py", line 131, in __exit__
    self.gen.throw(type, value, traceback)
  File "C:\Users\bramv\.virtualenvs\dutch-simplification-nbNdqK9u\lib\site-packages\nlp\builder.py", line 430, in incomplete_dir
    shutil.rmtree(tmp_dir)
  File "c:\users\bramv\appdata\local\programs\python\python38\lib\shutil.py", line 737, in rmtree
    return _rmtree_unsafe(path, onerror)
  File "c:\users\bramv\appdata\local\programs\python\python38\lib\shutil.py", line 615, in _rmtree_unsafe
    onerror(os.unlink, fullname, sys.exc_info())
  File "c:\users\bramv\appdata\local\programs\python\python38\lib\shutil.py", line 613, in _rmtree_unsafe
    os.unlink(fullname)
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\bramv\\.cache\\huggingface\\datasets\\text\\default-d18f9b6611eb8e16\\0.0.0\\3a79870d85f1982d6a2af884fde86a71c771747b4b161fd302d28ad22adf985b.incomplete\\text-train.arrow'
```

Hints from issue discussion:

Commit message: allow writer to write empty tables

Patch to review:
diff --git a/datasets/csv/csv.py b/datasets/csv/csv.py
index 27e42b04174..8e5d884bf1c 100644
--- a/datasets/csv/csv.py
+++ b/datasets/csv/csv.py
@@ -56,6 +56,7 @@ def _info(self):
     def _split_generators(self, dl_manager):
         """ We handle string, list and dicts in datafiles
         """
+        assert bool(self.config.data_files), "At least one data file must be specified, but got data_files={}".format(self.config.data_files)
         data_files = dl_manager.download_and_extract(self.config.data_files)
         if isinstance(data_files, (str, list, tuple)):
             files = data_files
diff --git a/datasets/json/json.py b/datasets/json/json.py
index c6b28de91af..0a9cdf12dd4 100644
--- a/datasets/json/json.py
+++ b/datasets/json/json.py
@@ -43,6 +43,7 @@ def _info(self):
     def _split_generators(self, dl_manager):
         """ We handle string, list and dicts in datafiles
         """
+        assert bool(self.config.data_files), "At least one data file must be specified, but got data_files={}".format(self.config.data_files)
         data_files = dl_manager.download_and_extract(self.config.data_files)
         if isinstance(data_files, (str, list, tuple)):
             files = data_files
diff --git a/datasets/pandas/pandas.py b/datasets/pandas/pandas.py
index 77648cd58cb..1156939f59b 100644
--- a/datasets/pandas/pandas.py
+++ b/datasets/pandas/pandas.py
@@ -13,6 +13,7 @@ def _info(self):
     def _split_generators(self, dl_manager):
         """ We handle string, list and dicts in datafiles
         """
+        assert bool(self.config.data_files), "At least one data file must be specified, but got data_files={}".format(self.config.data_files)
         data_files = dl_manager.download_and_extract(self.config.data_files)
         if isinstance(data_files, (str, list, tuple)):
             files = data_files
diff --git a/datasets/text/text.py b/datasets/text/text.py
index bbcdcf061aa..05e4f382938 100644
--- a/datasets/text/text.py
+++ b/datasets/text/text.py
@@ -11,6 +11,7 @@ def _split_generators(self, dl_manager):
             If str or List[str], then the dataset returns only the 'train' split.
             If dict, then keys should be from the `nlp.Split` enum.
         """
+        assert bool(self.config.data_files), "At least one data file must be specified, but got data_files={}".format(self.config.data_files)
         data_files = dl_manager.download_and_extract(self.config.data_files)
         if isinstance(data_files, (str, list, tuple)):
             files = data_files
diff --git a/src/nlp/arrow_reader.py b/src/nlp/arrow_reader.py
index ec45c7273fb..d862542650a 100644
--- a/src/nlp/arrow_reader.py
+++ b/src/nlp/arrow_reader.py
@@ -288,7 +288,8 @@ def _get_dataset_from_filename(self, filename_skip_take):
         mmap = pa.memory_map(filename)
         f = pa.ipc.open_stream(mmap)
         pa_table = f.read_all()
-        if skip is not None and take is not None:
+        # here we don't want to slice an empty table, or it may segfault
+        if skip is not None and take is not None and not (skip == 0 and take == len(pa_table)):
             pa_table = pa_table.slice(skip, take)
         return pa_table
 
@@ -317,7 +318,8 @@ def _get_dataset_from_filename(self, filename_skip_take):
             filename_skip_take["take"] if "take" in filename_skip_take else None,
         )
         pa_table = pa.parquet.read_table(filename, memory_map=True)
-        if skip is not None and take is not None:
+        # here we don't want to slice an empty table, or it may segfault
+        if skip is not None and take is not None and not (skip == 0 and take == len(pa_table)):
             pa_table = pa_table.slice(skip, take)
         return pa_table
 
diff --git a/src/nlp/arrow_writer.py b/src/nlp/arrow_writer.py
index ec4555ce5a9..776ae177d44 100644
--- a/src/nlp/arrow_writer.py
+++ b/src/nlp/arrow_writer.py
@@ -288,6 +288,11 @@ def write_table(self, pa_table: pa.Table, writer_batch_size: Optional[int] = Non
 
     def finalize(self, close_stream=True):
         self.write_on_file()
+        if self.pa_writer is None:
+            if self._schema is not None:
+                self._build_writer(self._schema)
+            else:
+                raise ValueError("Please pass `features` or at least one example when writing data")
         self.pa_writer.close()
         if close_stream:
             self.stream.close()


Code review comments:
diff_hunk: @@ -11,6 +11,7 @@ def _split_generators(self, dl_manager):
             If str or List[str], then the dataset returns only the 'train' split.
             If dict, then keys should be from the `nlp.Split` enum.
         """
+        assert bool(self.config.data_files), "At least one data file must be specified, but got data_files={}".format(self.config.data_files)
path: datasets/text/text.py
text: @user1:
Let's merge the PR on multi-threaded `text` and add it in the new script before?

@user2:
Be careful by putting all these checks in `assert`s - that's not what they should be used for. ValueErrors or filenotfound errors might be better. the issue with using asserts everywhere is that users will miss out on these error messages when they run their scripts in optimized mode, which turns off asserts (which are basically debug statements). This is true for the whole code base - using asserts to check arguments is not always what you'd want from a usability perspective.

I'd change it to:

```python
if not self.config.data_files:
    raise ValueError(f"At least one data file must be specified, but got data_files={self.config.data_files}")
```

@author:
Yes indeed I'll change that