Added on_error to Resource/Package

frictionless/package.py

@@ -36,6 +36,7 @@ class Package(Metadata):
         profile? (str): profile name like 'data-package'
         basepath? (str): a basepath of the package
         trusted? (bool): don't raise on unsafe paths
+        on_error? (ignore|warn|raise): behaviour if there is an error
 
     Raises:
         FrictionlessException: raise any error that occurs during the process
@@ -53,6 +54,7 @@ def __init__(
         profile=None,
         basepath=None,
         trusted=None,
+        on_error="ignore",
     ):
 
         # Handle zip
@@ -67,8 +69,15 @@ def __init__(
         self.setinitial("profile", profile)
         self.__basepath = basepath or helpers.detect_basepath(descriptor)
         self.__trusted = trusted
+        self.__on_error = on_error
         super().__init__(descriptor)
 
+    def __setattr__(self, name, value):
+        if name == "on_error":
+            self.__on_error = value
+            return
+        super().__setattr__(name, value)
+
     @Metadata.property
     def name(self):
         """
@@ -109,6 +118,15 @@ def profile(self):
         """
         return self.get("profile", config.DEFAULT_PACKAGE_PROFILE)
 
+    @property
+    def on_error(self):
+        """
+        Returns:
+            ignore|warn|raise: on error bahaviour
+        """
+        assert self.__on_error in ["ignore", "warn", "raise"]
+        return self.__on_error
+
     # Resources
 
     @Metadata.property
@@ -422,9 +440,12 @@ def metadata_process(self):
                         resource,
                         basepath=self.__basepath,
                         trusted=self.__trusted,
+                        on_error=self.__on_error,
                         package=self,
                     )
                     list.__setitem__(resources, index, resource)
+                # NOTE: should we sync basepath/trusted also here?
+                resource.on_error = self.__on_error
             if not isinstance(resources, helpers.ControlledList):
                 resources = helpers.ControlledList(resources)
                 resources.__onchange__(self.metadata_process)

frictionless/resource.py

@@ -42,6 +42,7 @@ class Resource(Metadata):
         profile? (str): resource profile
         basepath? (str): resource basepath
         trusted? (bool): don't raise on unsage paths
+        on_error? (ignore|warn|raise): behaviour if there is an error
         package? (Package): resource package
 
     Raises:
@@ -69,6 +70,7 @@ def __init__(
         profile=None,
         basepath=None,
         trusted=False,
+        on_error="ignore",
         package=None,
     ):
 
@@ -93,6 +95,7 @@ def __init__(
         self.setinitial("profile", profile)
         self.__basepath = basepath or helpers.detect_basepath(descriptor)
         self.__trusted = trusted
+        self.__on_error = on_error
         self.__package = package
         super().__init__(descriptor)
 
@@ -101,6 +104,12 @@ def __init__(
         if hashing != config.DEFAULT_HASHING:
             self["hashing"] = hashing
 
+    def __setattr__(self, name, value):
+        if name == "on_error":
+            self.__on_error = value
+            return
+        super().__setattr__(name, value)
+
     @Metadata.property
     def name(self):
         """
@@ -286,21 +295,6 @@ def compression_path(self):
         """
         return self.get("compressionPath")
 
-    @Metadata.property
-    def stats(self):
-        """
-        Returns
-            dict?: resource stats
-        """
-        stats = {}
-        for name in ["hash", "bytes", "rows"]:
-            value = self.get(name)
-            if value is not None:
-                if name == "hash":
-                    value = helpers.parse_resource_hash(value)[1]
-                stats[name] = value
-        return stats
-
     @Metadata.property
     def dialect(self):
         """
@@ -341,6 +335,30 @@ def profile(self):
         """
         return self.get("profile", config.DEFAULT_RESOURCE_PROFILE)
 
+    @property
+    def on_error(self):
+        """
+        Returns:
+            ignore|warn|raise: on error bahaviour
+        """
+        assert self.__on_error in ["ignore", "warn", "raise"]
+        return self.__on_error
+
+    @Metadata.property
+    def stats(self):
+        """
+        Returns
+            dict?: resource stats
+        """
+        stats = {}
+        for name in ["hash", "bytes", "rows"]:
+            value = self.get(name)
+            if value is not None:
+                if name == "hash":
+                    value = helpers.parse_resource_hash(value)[1]
+                stats[name] = value
+        return stats
+
     # Expand
 
     def expand(self):
@@ -724,6 +742,7 @@ def to_table(self, **options):
         options.setdefault("compression_path", self.compression_path)
         options.setdefault("dialect", self.dialect)
         options.setdefault("schema", self.schema)
+        options.setdefault("on_error", self.__on_error)
         if "lookup" not in options:
             options["lookup"] = self.read_lookup()
         return Table(**options)

frictionless/table.py

@@ -107,7 +107,7 @@ class Table:
             It defaults to `['']`
 
         on_error? (ignore|warn|raise): Define behaviour if there is an error in the
-            header or rows during a `table.read_rows` call.
+            header or rows during the reading rows process.
             It defaults to `ignore`.
 
         lookup? (dict): The lookup is a special object providing relational information.
@@ -147,9 +147,6 @@ def __init__(
         lookup=None,
     ):
 
-        # Validate arguments
-        assert on_error in ["ignore", "warn", "raise"]
-
         # Update source
         if isinstance(source, Path):
             source = str(source)
@@ -191,6 +188,9 @@ def __init__(
         self.__on_error = on_error
         self.__lookup = lookup
 
+        # Set error handler
+        self.on_error = on_error
+
         # Create file
         self.__file = File(
             source=source,
@@ -205,6 +205,12 @@ def __init__(
             query=query,
         )
 
+    def __setattr__(self, name, value):
+        if name == "on_error":
+            self.__on_error = value
+            return
+        super().__setattr__(name, value)
+
     def __enter__(self):
         if self.closed:
             self.open()
@@ -313,6 +319,24 @@ def schema(self):
         """
         return self.__schema
 
+    @property
+    def on_error(self):
+        """
+        Returns:
+            ignore|warn|raise: on error bahaviour
+        """
+        assert self.__on_error in ["ignore", "warn", "raise"]
+        return self.__on_error
+
+    # TODO: use property wrapper to make it shorter
+    @on_error.setter
+    def on_error(self, value):
+        """
+        Parameters:
+            value (ignore|warn|raise): on error bahaviour
+        """
+        self.__on_error = value
+
     @property
     def header(self):
         """
@@ -415,7 +439,8 @@ def closed(self):
         """
         return self.__parser is None
 
-    # Read
+        # Read
+        return self.__on_error
 
     def read_data(self):
         """Read data stream into memory

tests/test_package.py

@@ -2,7 +2,7 @@
 import json
 import zipfile
 import pytest
-from frictionless import Package, exceptions
+from frictionless import Package, Resource, exceptions
 
 
 # General
@@ -500,7 +500,60 @@ def test_package_compression_explicit_zip():
 
 # Integrity
 
-INTEGRITY_DESCRIPTOR = {
+
+def test_resource_integrity_on_error():
+    package = Package(resources=[Resource(path="data/invalid.csv")])
+    resource = package.resources[0]
+    assert package.on_error == "ignore"
+    assert resource.on_error == "ignore"
+    assert resource.read_rows()
+
+
+def test_resource_integrity_on_error_header_warn():
+    data = [["name"], [1], [2], [3]]
+    schema = {"fields": [{"name": "bad"}]}
+    package = Package(resources=[Resource(data=data, schema=schema)], on_error="warn")
+    resource = package.resources[0]
+    assert package.on_error == "warn"
+    assert resource.on_error == "warn"
+    with pytest.warns(UserWarning):
+        resource.read_rows()
+
+
+def test_resource_integrity_on_error_header_raise():
+    data = [["name"], [1], [2], [3]]
+    schema = {"fields": [{"name": "bad"}]}
+    package = Package({"resources": [{"data": data, "schema": schema}]}, on_error="raise")
+    resource = package.resources[0]
+    assert package.on_error == "raise"
+    assert resource.on_error == "raise"
+    with pytest.raises(exceptions.FrictionlessException):
+        resource.read_rows()
+
+
+def test_resource_integrity_on_error_row_warn():
+    data = [["name"], [1], [2], [3]]
+    schema = {"fields": [{"type": "string"}]}
+    package = Package(resources=[Resource(data=data, schema=schema)], on_error="warn")
+    resource = package.resources[0]
+    assert package.on_error == "warn"
+    assert resource.on_error == "warn"
+    with pytest.warns(UserWarning):
+        resource.read_rows()
+
+
+def test_resource_integrity_on_error_row_raise():
+    data = [["name"], [1], [2], [3]]
+    schema = {"fields": [{"type": "string"}]}
+    package = Package({"resources": [{"data": data, "schema": schema}]}, on_error="raise")
+    resource = package.resources[0]
+    assert package.on_error == "raise"
+    assert resource.on_error == "raise"
+    with pytest.raises(exceptions.FrictionlessException):
+        resource.read_rows()
+
+
+DESCRIPTOR_FK = {
     "resources": [
         {
             "name": "main",
@@ -538,8 +591,8 @@ def test_package_compression_explicit_zip():
 }
 
 
-def test_package_integrity():
-    package = Package(INTEGRITY_DESCRIPTOR)
+def test_package_integrity_foreign_key():
+    package = Package(DESCRIPTOR_FK)
     resource = package.get_resource("main")
     rows = resource.read_rows()
     assert rows[0].valid
@@ -553,7 +606,7 @@ def test_package_integrity():
 
 
 def test_package_integrity_foreign_key_invalid():
-    package = Package(INTEGRITY_DESCRIPTOR)
+    package = Package(DESCRIPTOR_FK)
     package.resources[1].data[3][0] = "bad"
     resource = package.get_resource("main")
     rows = resource.read_rows()
@@ -568,7 +621,7 @@ def test_package_integrity_foreign_key_invalid():
 
 
 def test_package_integrity_foreign_key_self_reference():
-    package = Package(INTEGRITY_DESCRIPTOR)
+    package = Package(DESCRIPTOR_FK)
     package.resources[0].schema.foreign_keys = [
         {"fields": "parent_id", "reference": {"resource": "", "fields": "id"}}
     ]
@@ -580,7 +633,7 @@ def test_package_integrity_foreign_key_self_reference():
 
 
 def test_package_integrity_foreign_key_self_reference_invalid():
-    package = Package(INTEGRITY_DESCRIPTOR)
+    package = Package(DESCRIPTOR_FK)
     package.resources[0].data[2][0] = "0"
     package.resources[0].schema.foreign_keys = [
         {"fields": "parent_id", "reference": {"resource": "", "fields": "id"}}
@@ -593,7 +646,7 @@ def test_package_integrity_foreign_key_self_reference_invalid():
 
 
 def test_package_integrity_foreign_key_multifield():
-    package = Package(INTEGRITY_DESCRIPTOR)
+    package = Package(DESCRIPTOR_FK)
     package.resources[0].schema.foreign_keys = [
         {
             "fields": ["name", "surname"],
@@ -608,7 +661,7 @@ def test_package_integrity_foreign_key_multifield():
 
 
 def test_package_integrity_foreign_key_multifield_invalid():
-    package = Package(INTEGRITY_DESCRIPTOR)
+    package = Package(DESCRIPTOR_FK)
     package.resources[0].schema.foreign_keys = [
         {
             "fields": ["name", "surname"],
@@ -624,7 +677,7 @@ def test_package_integrity_foreign_key_multifield_invalid():
 
 
 def test_package_integrity_read_lookup():
-    package = Package(INTEGRITY_DESCRIPTOR)
+    package = Package(DESCRIPTOR_FK)
     resource = package.get_resource("main")
     lookup = resource.read_lookup()
     assert lookup == {"people": {("firstname",): {("Walter",), ("Alex",), ("John",)}}}