Repository: deepset-ai/haystack
Pull request: Make FileTypeClassifier more flexible
This PR proposes an enhancement of `FileTypeClassifier` to make it accept more extensions rather than the 5 hardcoded ones.

Closes #1977


Problem statement:
Using Tika Converter
**Question**
How can I run haystack docker container with default file converter as tika?

**Additional context**
And also built in tika converter can convert and process ppt files? If no how can I add tika ppt converter to haystack? To process also from ppt files.

Hints from issue discussion:
Hi @arater!
To your first question: As the `TikaConverter` implemented in haystack supports all data formats compatible with Tika (you can find them [here](https://tika.apache.org/2.2.1/formats.html)), using it with a `.ppt` file shouldn't be a problem.

To your second question: To load a file using `TikaConverter`, you need to adjust the indexing pipeline which is defined in [`pipelines.yaml`](https://github.com/deepset-ai/haystack/blob/00dc30ae547bd4a55e035e51cd0bfe50f2e5b47d/rest_api/pipeline/pipelines.yaml#L39-L53). This file contains the following lines:
```
  - name: indexing
    type: Indexing
    nodes:
      - name: FileTypeClassifier
        inputs: [File]
      - name: TextFileConverter
        inputs: [FileTypeClassifier.output_1]
      - name: PDFFileConverter
        inputs: [FileTypeClassifier.output_2]
      - name: Preprocessor
        inputs: [PDFFileConverter, TextFileConverter]
      - name: Retriever
        inputs: [Preprocessor]
      - name: DocumentStore
        inputs: [Retriever]
```
To use `TikaConverter` instead just replace the snippet by the following:
```
  - name: indexing
    type: Indexing
    nodes:
      - name: TikaConverter
        inputs: [File]
      - name: Preprocessor
        inputs: [TikaConverter]
      - name: Retriever
        inputs: [Preprocessor]
      - name: DocumentStore
        inputs: [Retriever]
```
Additionally, you also need to start a Tika server. This can be done by running the following command: `docker run -d --name tika -p 9998:9998 apache/tika:1.24.1`. After that, you should be able to upload your files either using the demo interface or using the REST api.
I converted the tika converter but when I run docker run for tika it says it runs and gives output below.
<img width="694" alt="Screen Shot 2022-01-17 at 00 02 00" src="https://user-images.githubusercontent.com/12005291/149681740-1e50ac2f-d692-4f26-aaea-96987dc8d78c.png">

From the ui when I want to upload a file I could not choose ppt. Why this is happened? And also could you give postman request example for file-upload post request because I can upload other files from ui but not from swagger.
 
The output you are getting for Tika is actually right. It is just returning an identifier for your container.

The demo is pretty so the accepted file types are hard coded. It should be enough just to add the new file types in [this file](https://github.com/deepset-ai/haystack/blob/0cca2b97cd63d61a2f50696b3ddb7f92fd677fdd/ui/webapp.py#L87).
For example, to accept .ppt in the UI, the line would look like this:
`        data_files = st.sidebar.file_uploader("", type=["pdf", "txt", "docx", "ppt"], accept_multiple_files=True)`

Here is a postman file upload example:
![image](https://user-images.githubusercontent.com/17650521/149763016-86a59ede-da25-4a58-8c72-09765f99b458.png)

Hello @arater, just checking if @MichelBartels's suggestion helped, or if you have more questions :slightly_smiling_face: If not I'll close the issue in a few days
After adding the code you wrote and even running the tika as docker I am getting the error below while uploading pptx or ppt file.

<img width="850" alt="Screen Shot 2022-01-20 at 21 55 26" src="https://user-images.githubusercontent.com/12005291/150420769-6526c22b-4de0-4acc-a2dc-0074dc23a248.png">

Hello @arater, thank you for the stacktrace. Indeed it seems that there are some hardcoded extensions in the `FileTypeClassifier` class, which I believe is wrong. I'm going to make a small PR to convert this exception into a warning and see if it works.
Is there any progress about this?
Hello @arater, sorry for the delay. Yes, there's been some developments and I believe I might have a fix ready today. Keep an eye on the linked PR :slightly_smiling_face:

Commit message: Fix mypy

Patch to review:
diff --git a/haystack/nodes/file_classifier/file_type.py b/haystack/nodes/file_classifier/file_type.py
index 306c5d12f2..1a1c4349d4 100644
--- a/haystack/nodes/file_classifier/file_type.py
+++ b/haystack/nodes/file_classifier/file_type.py
@@ -1,40 +1,72 @@
+from multiprocessing.sharedctypes import Value
 from typing import List, Union
 from pathlib import Path
 from haystack.nodes.base import BaseComponent
 
 
+DEFAULT_TYPES = ["txt", "pdf", "md", "docx", "html"]
+
+
 class FileTypeClassifier(BaseComponent):
     """
     Route files in an Indexing Pipeline to corresponding file converters.
     """
-    outgoing_edges = 5
+    outgoing_edges = 10
+
+    def __init__(self, supported_types: List[str] = DEFAULT_TYPES):
+        """
+        Node that sends out files on a different output edge depending on their extension.
+
+        :param supported_types: the file types that this node can distinguish.
+            Note that it's limited to a maximum of 10 outgoing edges, which 
+            correspond each to a file extension. Such extension are, by default
+            `txt`, `pdf`, `md`, `docx`, `html`. Lists containing more than 10
+            elements will not be allowed. Lists with duplicate elements will 
+            also be rejected.
+        """
+        if len(supported_types) > 5:
+            raise ValueError("supported_types can't have more than 5 values.")
+        if len(set(supported_types)) != len(supported_types):
+            raise ValueError("supported_types can't contain duplicate values.")
 
-    def __init__(self):
-        self.set_config()
+        self.set_config(supported_types=supported_types)
+        self.supported_types = supported_types
 
-    def _get_files_extension(self, file_paths: list) -> set:
+    def _get_extension(self, file_paths: List[Path]) -> str:
         """
-        Return the file extensions
-        :param file_paths:
-        :return: set
+        Return the extension found in the given list of files.
+        Also makes sure that all files have the same extension.
+        If this is not true, it throws an exception.
+
+        :param file_paths: the paths to extract the extension from
+        :return: a set of strings with all the extensions (without duplicates)
         """
-        return {file_path.suffix.lstrip(".") for file_path in file_paths}
+        extension = file_paths[0].suffix
+
+        for path in file_paths:
+            if path.suffix != extension:
+                raise ValueError(f"Multiple files types are not allowed at once.")
 
-    def run(self, file_paths: Union[Path, List[Path]]):  # type: ignore
+        return extension.lstrip(".")
+
+    def run(self, file_paths: Union[Path, List[Path], str, List[str], List[Union[Path, str]]]):  # type: ignore
         """
-        Return the output based on file extension
+        Sends out files on a different output edge depending on their extension.
+
+        :param file_paths: paths to route on different edges.
         """
-        if isinstance(file_paths, Path):
+        if not isinstance(file_paths, list):
             file_paths = [file_paths]
 
-        extension: set = self._get_files_extension(file_paths)
-        if len(extension) > 1:
-            raise ValueError(f"Multiple files types are not allowed at once.")
+        paths = [Path(path) for path in file_paths]
 
-        output = {"file_paths": file_paths}
-        ext: str = extension.pop()
+        output = {"file_paths": paths}
+        extension = self._get_extension(paths)
         try:
-            index = ["txt", "pdf", "md", "docx", "html"].index(ext) + 1
-            return output, f"output_{index}"
+            index = self.supported_types.index(extension) + 1
         except ValueError:
-            raise Exception(f"Files with an extension '{ext}' are not supported.")
+            raise ValueError(f"Files of type '{extension}' are not supported. "
+                            f"The supported types are: {self.supported_types}. "
+                            "Consider using the 'supported_types' parameter to "
+                            "change the types accepted by this node.")
+        return output, f"output_{index}"
diff --git a/test/conftest.py b/test/conftest.py
index 46548144d9..41179c0aa3 100644
--- a/test/conftest.py
+++ b/test/conftest.py
@@ -141,6 +141,14 @@ def pytest_collection_modifyitems(config,items):
                 item.add_marker(skip_docstore)
 
 
+@pytest.fixture
+def tmpdir(tmpdir):
+    """
+    Makes pytest's tmpdir fixture fully compatible with pathlib
+    """
+    return Path(tmpdir)
+
+
 @pytest.fixture(scope="function", autouse=True)
 def gc_cleanup(request):
     """
diff --git a/test/test_filetype_classifier.py b/test/test_filetype_classifier.py
new file mode 100644
index 0000000000..086ea4214b
--- /dev/null
+++ b/test/test_filetype_classifier.py
@@ -0,0 +1,57 @@
+import pytest
+from pathlib import Path
+from haystack.nodes.file_classifier.file_type import FileTypeClassifier, DEFAULT_TYPES
+
+
+def test_filetype_classifier_single_file(tmpdir):
+    node = FileTypeClassifier()
+    test_files = [tmpdir/f"test.{extension}" for extension in DEFAULT_TYPES]
+
+    for edge_index, test_file in enumerate(test_files):
+        output, edge = node.run(test_file)
+        assert edge == f"output_{edge_index+1}"
+        assert output == {"file_paths": [test_file]}
+
+
+def test_filetype_classifier_many_files(tmpdir):
+    node = FileTypeClassifier()
+
+    for edge_index, extension in enumerate(DEFAULT_TYPES):
+        test_files = [tmpdir/f"test_{idx}.{extension}" for idx in range(10)]
+
+        output, edge = node.run(test_files)
+        assert edge == f"output_{edge_index+1}"
+        assert output == {"file_paths": test_files}
+
+
+def test_filetype_classifier_many_files_mixed_extensions(tmpdir):
+    node = FileTypeClassifier()
+    test_files = [tmpdir/f"test.{extension}" for extension in DEFAULT_TYPES]
+
+    with pytest.raises(ValueError):
+        node.run(test_files)
+    
+
+def test_filetype_classifier_unsupported_extension(tmpdir):
+    node = FileTypeClassifier()
+    test_file = tmpdir/f"test.really_weird_extension"
+    with pytest.raises(ValueError):
+        node.run(test_file)
+
+
+def test_filetype_classifier_custom_extensions(tmpdir):
+    node = FileTypeClassifier(supported_types=["my_extension"])
+    test_file = tmpdir/f"test.my_extension"
+    output, edge = node.run(test_file)
+    assert edge == f"output_1"
+    assert output == {"file_paths": [test_file]}
+
+
+def test_filetype_classifier_too_many_custom_extensions():
+    with pytest.raises(ValueError):
+        FileTypeClassifier(supported_types=[f"my_extension_{idx}" for idx in range(20)])
+
+
+def test_filetype_classifier_duplicate_custom_extensions():
+    with pytest.raises(ValueError):
+        FileTypeClassifier(supported_types=[f"my_extension", "my_extension"])


Code review comments:
diff_hunk: @@ -1,40 +1,72 @@
+from multiprocessing.sharedctypes import Value
 from typing import List, Union
 from pathlib import Path
 from haystack.nodes.base import BaseComponent
 
 
+DEFAULT_TYPES = ["txt", "pdf", "md", "docx", "html"]
+
+
 class FileTypeClassifier(BaseComponent):
     """
     Route files in an Indexing Pipeline to corresponding file converters.
     """
-    outgoing_edges = 5
+    outgoing_edges = 10
+
+    def __init__(self, supported_types: List[str] = DEFAULT_TYPES):
+        """
+        Node that sends out files on a different output edge depending on their extension.
+
+        :param supported_types: the file types that this node can distinguish.
+            Note that it's limited to a maximum of 10 outgoing edges, which 
+            correspond each to a file extension. Such extension are, by default
+            `txt`, `pdf`, `md`, `docx`, `html`. Lists containing more than 10
+            elements will not be allowed. Lists with duplicate elements will 
+            also be rejected.
+        """
+        if len(supported_types) > 5:
path: haystack/nodes/file_classifier/file_type.py
text: @user1:
should be 10 instead of 5 if I am not mistaken as the docstring also says "Lists containing more than 10 elements will not be allowed"

diff_hunk: @@ -1,40 +1,72 @@
+from multiprocessing.sharedctypes import Value
 from typing import List, Union
 from pathlib import Path
 from haystack.nodes.base import BaseComponent
 
 
+DEFAULT_TYPES = ["txt", "pdf", "md", "docx", "html"]
+
+
 class FileTypeClassifier(BaseComponent):
     """
     Route files in an Indexing Pipeline to corresponding file converters.
     """
-    outgoing_edges = 5
+    outgoing_edges = 10
+
+    def __init__(self, supported_types: List[str] = DEFAULT_TYPES):
+        """
+        Node that sends out files on a different output edge depending on their extension.
+
+        :param supported_types: the file types that this node can distinguish.
+            Note that it's limited to a maximum of 10 outgoing edges, which 
+            correspond each to a file extension. Such extension are, by default
+            `txt`, `pdf`, `md`, `docx`, `html`. Lists containing more than 10
+            elements will not be allowed. Lists with duplicate elements will 
+            also be rejected.
+        """
+        if len(supported_types) > 5:
+            raise ValueError("supported_types can't have more than 5 values.")
+        if len(set(supported_types)) != len(supported_types):
+            raise ValueError("supported_types can't contain duplicate values.")
 
-    def __init__(self):
-        self.set_config()
+        self.set_config(supported_types=supported_types)
+        self.supported_types = supported_types
 
-    def _get_files_extension(self, file_paths: list) -> set:
+    def _get_extension(self, file_paths: List[Path]) -> str:
         """
-        Return the file extensions
-        :param file_paths:
-        :return: set
+        Return the extension found in the given list of files.
+        Also makes sure that all files have the same extension.
+        If this is not true, it throws an exception.
+
+        :param file_paths: the paths to extract the extension from
+        :return: a set of strings with all the extensions (without duplicates)
         """
-        return {file_path.suffix.lstrip(".") for file_path in file_paths}
+        extension = file_paths[0].suffix
+
+        for path in file_paths:
+            if path.suffix != extension:
+                raise ValueError(f"Multiple files types are not allowed at once.")
path: haystack/nodes/file_classifier/file_type.py
text: @user1:
typo in "Multiple file**s** types"

diff_hunk: @@ -1,40 +1,72 @@
+from multiprocessing.sharedctypes import Value
 from typing import List, Union
 from pathlib import Path
 from haystack.nodes.base import BaseComponent
 
 
+DEFAULT_TYPES = ["txt", "pdf", "md", "docx", "html"]
+
+
 class FileTypeClassifier(BaseComponent):
     """
     Route files in an Indexing Pipeline to corresponding file converters.
     """
-    outgoing_edges = 5
+    outgoing_edges = 10
+
+    def __init__(self, supported_types: List[str] = DEFAULT_TYPES):
+        """
+        Node that sends out files on a different output edge depending on their extension.
+
+        :param supported_types: the file types that this node can distinguish.
+            Note that it's limited to a maximum of 10 outgoing edges, which 
+            correspond each to a file extension. Such extension are, by default
+            `txt`, `pdf`, `md`, `docx`, `html`. Lists containing more than 10
+            elements will not be allowed. Lists with duplicate elements will 
+            also be rejected.
+        """
+        if len(supported_types) > 5:
+            raise ValueError("supported_types can't have more than 5 values.")
path: haystack/nodes/file_classifier/file_type.py
text: @user1:
also 10 instead of 5