Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ingest_service.py to fix issue Error: 'utf-8' codec can't decode #1171

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion private_gpt/server/ingest/ingest_service.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import tempfile
import chardet # Chardet must be put in requirements or manually install with pip install chardet
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should add the dependency,

poetry add chardet

from pathlib import Path
from typing import TYPE_CHECKING, Any, AnyStr

Expand Down Expand Up @@ -77,7 +78,10 @@ def ingest(self, file_name: str, file_data: AnyStr | Path) -> list[IngestedDoc]:
# Read as a plain text
string_reader = StringIterableReader()
if isinstance(file_data, Path):
text = file_data.read_text()
with open(file_data, 'rb') as f2:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to give this variables proper names. Maybe file_handle instead of f2 and charset instead of result2

Copy link
Contributor

@lopagela lopagela Nov 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would go even further, and could you also try to avoid to read the file in it's entirety to detect the charset?

More information in chardet documentation: https://chardet.readthedocs.io/en/latest/usage.html#advanced-usage

While the existing implementation work, it is suboptimal in the sense that it reads the file in its entirety before re-reading it?


One could also do the following: read the file in binary mode, store it in text_bin, and then run chardet on it, so that we have text = text_bin.decode(encoding=chardet.detect(text_bin))

This is doing only a single read, and it re-uses the buffer in memory instead of re-reading it.

result2 = chardet.detect(f2.read())
text = file_data.read_text(encoding=result2['encoding'])
#text = file_data.read_text()
documents = string_reader.load_data([text])
elif isinstance(file_data, bytes):
documents = string_reader.load_data([file_data.decode("utf-8")])
Expand Down
Loading