Repository: frictionlessdata/frictionless-py

Issue:
Encoding detection for a resource doesn't always work
# Overview

After `loader.buffer` introduction we got some regression in encoding detection. See xfailed tests

Discussion:

Patch:
diff --git a/frictionless/detector.py b/frictionless/detector.py
index 5fc918b6..7416ec77 100644
--- a/frictionless/detector.py
+++ b/frictionless/detector.py
@@ -1,3 +1,4 @@
+import codecs
 import chardet
 from copy import copy, deepcopy
 from .exception import FrictionlessException
@@ -98,7 +99,7 @@ class Detector:
 
     # Detect
 
-    def detect_encoding(self, buffer):
+    def detect_encoding(self, buffer, *, encoding=None):
         """Detect encoding from buffer
 
         Parameters:
@@ -107,15 +108,37 @@ class Detector:
         Returns:
             str: encoding
         """
+
+        # Use defined
         if self.__encoding_function:
             return self.__encoding_function(buffer)
-        result = chardet.detect(buffer)
-        encoding = result["encoding"] or config.DEFAULT_ENCODING
-        confidence = result["confidence"] or 0
-        if confidence < self.__encoding_confidence:
-            encoding = config.DEFAULT_ENCODING
-        if encoding == "ascii":
-            encoding = config.DEFAULT_ENCODING
+
+        # Detect encoding
+        if not encoding:
+            result = chardet.detect(buffer)
+            encoding = result["encoding"] or config.DEFAULT_ENCODING
+            confidence = result["confidence"] or 0
+            if confidence < self.__encoding_confidence:
+                encoding = config.DEFAULT_ENCODING
+            if encoding == "ascii":
+                encoding = config.DEFAULT_ENCODING
+            if encoding is None:
+                encoding = self.resource.detector.detect_encoding(buffer)
+
+        # Normalize encoding
+        encoding = codecs.lookup(encoding).name
+        # Work around for incorrect inferion of utf-8-sig encoding
+        if encoding == "utf-8":
+            if buffer.startswith(codecs.BOM_UTF8):
+                encoding = "utf-8-sig"
+        # Use the BOM stripping name (without byte-order) for UTF-16 encodings
+        elif encoding == "utf-16-be":
+            if buffer.startswith(codecs.BOM_UTF16_BE):
+                encoding = "utf-16"
+        elif encoding == "utf-16-le":
+            if buffer.startswith(codecs.BOM_UTF16_LE):
+                encoding = "utf-16"
+
         return encoding
 
     def detect_layout(self, sample, *, layout=None):
diff --git a/frictionless/loader.py b/frictionless/loader.py
index 65f3fc2a..a0254731 100644
--- a/frictionless/loader.py
+++ b/frictionless/loader.py
@@ -1,7 +1,6 @@
 import io
 import os
 import gzip
-import codecs
 import shutil
 import atexit
 import hashlib
@@ -83,6 +82,8 @@ class Loader:
         Returns:
             io.TextStream: resource text stream
         """
+        if not self.__text_stream:
+            self.__text_stream = self.read_text_stream()
         return self.__text_stream
 
     # Open/Close
@@ -92,7 +93,6 @@ class Loader:
         self.close()
         try:
             self.__byte_stream = self.read_byte_stream()
-            self.__text_stream = self.read_text_stream()
             return self
         except Exception:
             self.close()
@@ -123,14 +123,20 @@ class Loader:
         """
         try:
             byte_stream = self.read_byte_stream_create()
-            byte_stream = self.read_byte_stream_infer_stats(byte_stream)
+            byte_stream = self.read_byte_stream_process(byte_stream)
             byte_stream = self.read_byte_stream_decompress(byte_stream)
-        except IOError as exception:
-            error = errors.SchemeError(note=str(exception))
-            raise FrictionlessException(error)
+            buffer = self.read_byte_stream_buffer(byte_stream)
+            self.read_byte_stream_analyze(buffer)
+            self.__buffer = buffer
+        except (LookupError, UnicodeDecodeError) as exception:
+            error = errors.EncodingError(note=str(exception))
+            raise FrictionlessException(error) from exception
         except config.COMPRESSION_EXCEPTIONS as exception:
             error = errors.CompressionError(note=str(exception))
             raise FrictionlessException(error)
+        except IOError as exception:
+            error = errors.SchemeError(note=str(exception))
+            raise FrictionlessException(error)
         return byte_stream
 
     def read_byte_stream_create(self):
@@ -141,8 +147,8 @@ class Loader:
         """
         raise NotImplementedError()
 
-    def read_byte_stream_infer_stats(self, byte_stream):
-        """Infer byte stream stats
+    def read_byte_stream_process(self, byte_stream):
+        """Process byte stream
 
         Parameters:
             byte_stream (io.ByteStream): resource byte stream
@@ -150,8 +156,6 @@ class Loader:
         Returns:
             io.ByteStream: resource byte stream
         """
-        if not self.resource.get("stats"):
-            return byte_stream
         return ByteStreamWithStatsHandling(byte_stream, resource=self.resource)
 
     def read_byte_stream_decompress(self, byte_stream):
@@ -209,59 +213,40 @@ class Loader:
         note = f'compression "{self.resource.compression}" is not supported'
         raise FrictionlessException(errors.CompressionError(note=note))
 
-    def read_text_stream(self):
-        """Read text stream
+    def read_byte_stream_buffer(self, byte_stream):
+        """Buffer byte stream
+
+        Parameters:
+            byte_stream (io.ByteStream): resource byte stream
 
         Returns:
-            io.TextStream: resource text stream
+            bytes: buffer
         """
-        try:
-            self.read_text_stream_infer_encoding(self.byte_stream)
-        except (LookupError, UnicodeDecodeError) as exception:
-            error = errors.EncodingError(note=str(exception))
-            raise FrictionlessException(error) from exception
-        return self.read_text_stream_decode(self.byte_stream)
+        buffer = byte_stream.read(self.resource.detector.buffer_size)
+        buffer = buffer[: self.resource.detector.buffer_size]
+        byte_stream.seek(0)
+        return buffer
 
-    def read_text_stream_infer_encoding(self, byte_stream):
-        """Infer text stream encoding
+    def read_byte_stream_analyze(self, buffer):
+        """Detect metadta using sample
 
         Parameters:
-            byte_stream (io.ByteStream): resource byte stream
+            buffer (bytes): byte buffer
         """
         # We don't need a default encoding
         encoding = self.resource.get("encoding")
-        buffer = byte_stream.read(self.resource.detector.buffer_size)
-        buffer = buffer[: self.resource.detector.buffer_size]
-        byte_stream.seek(0)
-        if encoding is None:
-            encoding = self.resource.detector.detect_encoding(buffer)
-        encoding = codecs.lookup(encoding).name
-        # Work around for incorrect inferion of utf-8-sig encoding
-        if encoding == "utf-8":
-            if buffer.startswith(codecs.BOM_UTF8):
-                encoding = "utf-8-sig"
-        # Use the BOM stripping name (without byte-order) for UTF-16 encodings
-        elif encoding == "utf-16-be":
-            if buffer.startswith(codecs.BOM_UTF16_BE):
-                encoding = "utf-16"
-        elif encoding == "utf-16-le":
-            if buffer.startswith(codecs.BOM_UTF16_LE):
-                encoding = "utf-16"
+        encoding = self.resource.detector.detect_encoding(buffer, encoding=encoding)
         self.resource.encoding = encoding
-        self.__buffer = buffer
 
-    def read_text_stream_decode(self, byte_stream):
-        """Decode text stream
-
-        Parameters:
-            byte_stream (io.ByteStream): resource byte stream
+    def read_text_stream(self):
+        """Read text stream
 
         Returns:
-            text_stream (io.TextStream): resource text stream
+            io.TextStream: resource text stream
         """
         # NOTE: this solution might be improved using parser properties
         newline = "" if self.resource.format == "csv" else None
-        return io.TextIOWrapper(byte_stream, self.resource.encoding, newline=newline)
+        return io.TextIOWrapper(self.byte_stream, self.resource.encoding, newline=newline)
 
     # Write
 
diff --git a/frictionless/resource.py b/frictionless/resource.py
index fb08981a..6196e645 100644
--- a/frictionless/resource.py
+++ b/frictionless/resource.py
@@ -715,7 +715,6 @@ class Resource(Metadata):
         self["scheme"] = self.scheme
         self["format"] = self.format
         self["hashing"] = self.hashing
-        self["encoding"] = self.encoding
         if self.innerpath:
             self["innerpath"] = self.innerpath
         if self.compression:


Test patch:
diff --git a/tests/plugins/test_csv.py b/tests/plugins/test_csv.py
index 96f18cfa..2d485de0 100644
--- a/tests/plugins/test_csv.py
+++ b/tests/plugins/test_csv.py
@@ -114,7 +114,6 @@ def test_csv_parser_remote():
         ]
 
 
-@pytest.mark.xfail(reason="encoding")
 @pytest.mark.vcr
 def test_csv_parser_remote_non_ascii_url():
     source = "http://data.defra.gov.uk/ops/government_procurement_card/over_£500_GPC_apr_2013.csv"
diff --git a/tests/plugins/test_remote.py b/tests/plugins/test_remote.py
index 0540bfe6..76d4d615 100644
--- a/tests/plugins/test_remote.py
+++ b/tests/plugins/test_remote.py
@@ -18,7 +18,6 @@ def test_remote_loader():
         ]
 
 
-@pytest.mark.xfail(reason="encoding")
 @pytest.mark.vcr
 def test_remote_loader_latin1():
     # Github returns wrong encoding `utf-8`
diff --git a/tests/test_package.py b/tests/test_package.py
index d0823400..eddd32b9 100644
--- a/tests/test_package.py
+++ b/tests/test_package.py
@@ -697,7 +697,6 @@ def test_package_infer_multiple_paths():
     assert package.resources[1].path == "data2.csv"
 
 
-@pytest.mark.xfail(reason="encoding")
 def test_package_infer_non_utf8_file():
     package = Package("data/table-with-accents.csv")
     package.infer()
diff --git a/tests/test_resource.py b/tests/test_resource.py
index 79cccc9f..afe9fe64 100644
--- a/tests/test_resource.py
+++ b/tests/test_resource.py
@@ -519,7 +519,6 @@ def test_resource_encoding_explicit_latin1():
         ]
 
 
-@pytest.mark.xfail(reason="encoding")
 def test_resource_encoding_utf_16():
     # Bytes encoded as UTF-16 with BOM in platform order is detected
     bio = io.BytesIO(u"en,English\nja,日本語".encode("utf-16"))
@@ -705,7 +704,6 @@ def test_resource_compression_error_invalid_zip():
     assert error.note == "File is not a zip file"
 
 
-@pytest.mark.xfail(reason="encoding")
 @pytest.mark.skipif(sys.version_info < (3, 8), reason="Requires Python3.8+")
 def test_resource_compression_error_invalid_gz():
     source = b"id,filename\n\1,dump"
@@ -2511,10 +2509,8 @@ def test_resource_skip_blank_at_the_end_issue_bco_dmo_33():
         assert rows[1].cells == []
 
 
-@pytest.mark.xfail(reason="encoding")
 def test_resource_wrong_encoding_detection_issue_265():
     with Resource("data/accent.csv") as resource:
-        print(resource.encoding)
         assert resource.encoding == "iso8859-1"