Repository: frictionlessdata/frictionless-py

Issue:
Descriptors not being honored for some transforms - incorrect asserts or default values are causing issues
# Overview

Transforms like row_subset, row_sort, table_join are currently not working when using JSON/YAML pipelines or through the CLI. 

row_subset will not work because the assert is on the param and ignores the descriptors coming form yaml/json
```
    def __init__(self, descriptor=None, *, subset=None, field_name=None):
        assert subset in ["conflicts", "distinct", "duplicates", "unique"]
        self.setinitial("subset", subset)
        self.setinitial("fieldName", field_name)
        super().__init__(descriptor)
```

row_sort will not honor `reverse` because the base class init only sets descriptors using `setdefault` and reverse is always set.
```
    def __init__(self, descriptor=None, *, field_names=None, reverse=False):
        self.setinitial("fieldNames", field_names)
        self.setinitial("reverse", reverse)
        super().__init__(descriptor)
```

table_join is a combination of the above - assert ignores descriptors, mode is ignored because it has a default value
```
        assert mode in ["inner", "left", "right", "outer", "cross", "negate"]
        self.setinitial("resource", resource)
        self.setinitial("fieldName", field_name)
        self.setinitial("useHash", use_hash)
        self.setinitial("mode", mode)
        super().__init__(descriptor)
```

I'm pretty sure there are more transforms that run into these same issues. 
Some transforms resolve this by running
```
        for key, value in helpers.create_descriptor(**options).items():
            self.setinitial(key, value)
```

Will likely need to move the asserts to be based on `assert self.get('option') == X` to be after `super().__init(descriptor)__` or explicitly check descriptors if defined.


Discussion:

Patch:
diff --git a/frictionless/errors/cell.py b/frictionless/errors/cell.py
index c8455e4d..ac3c8d75 100644
--- a/frictionless/errors/cell.py
+++ b/frictionless/errors/cell.py
@@ -17,7 +17,7 @@ class CellError(RowError):
         field_number (int): field number
         field_position (int): field position
 
-    # Raises
+    Raises
         FrictionlessException: raise any error that occurs during the process
 
     """
diff --git a/frictionless/steps/cell.py b/frictionless/steps/cell.py
index a3bec82a..6d36fac1 100644
--- a/frictionless/steps/cell.py
+++ b/frictionless/steps/cell.py
@@ -52,7 +52,6 @@ class cell_fill(Step):
     code = "cell-fill"
 
     def __init__(self, descriptor=None, *, value=None, field_name=None, direction=None):
-        assert direction in [None, "down", "right", "left"]
         self.setinitial("value", value)
         self.setinitial("fieldName", field_name)
         self.setinitial("direction", direction)
@@ -85,7 +84,10 @@ class cell_fill(Step):
         "properties": {
             "fieldName": {"type": "string"},
             "value": {},
-            "direction": {},
+            "direction": {
+                "type": "string",
+                "enum": ["down", "right", "left"],
+            },
         },
     }
 
diff --git a/frictionless/steps/row.py b/frictionless/steps/row.py
index f2021fa9..d09e058f 100644
--- a/frictionless/steps/row.py
+++ b/frictionless/steps/row.py
@@ -137,7 +137,7 @@ class row_sort(Step):
 
     code = "row-sort"
 
-    def __init__(self, descriptor=None, *, field_names=None, reverse=False):
+    def __init__(self, descriptor=None, *, field_names=None, reverse=None):
         self.setinitial("fieldNames", field_names)
         self.setinitial("reverse", reverse)
         super().__init__(descriptor)
@@ -147,7 +147,7 @@ class row_sort(Step):
     def transform_resource(self, resource):
         table = resource.to_petl()
         field_names = self.get("fieldNames")
-        reverse = self.get("reverse")
+        reverse = self.get("reverse", False)
         resource.data = table.sort(field_names, reverse=reverse)
 
     # Metadata
@@ -198,7 +198,6 @@ class row_subset(Step):
     code = "row-subset"
 
     def __init__(self, descriptor=None, *, subset=None, field_name=None):
-        assert subset in ["conflicts", "distinct", "duplicates", "unique"]
         self.setinitial("subset", subset)
         self.setinitial("fieldName", field_name)
         super().__init__(descriptor)
@@ -224,7 +223,10 @@ class row_subset(Step):
         "type": "object",
         "required": ["subset"],
         "properties": {
-            "subset": {"type": "string"},
+            "subset": {
+                "type": "string",
+                "enum": ["conflicts", "distinct", "duplicates", "unique"],
+            },
             "fieldName": {"type": "string"},
         },
     }
@@ -243,7 +245,6 @@ class row_ungroup(Step):
         group_name=None,
         value_name=None,
     ):
-        assert selection in ["first", "last", "min", "max"]
         self.setinitial("selection", selection)
         self.setinitial("groupName", group_name)
         self.setinitial("valueName", value_name)
@@ -266,7 +267,10 @@ class row_ungroup(Step):
         "type": "object",
         "required": ["groupName", "selection"],
         "properties": {
-            "selection": {"type": "string"},
+            "selection": {
+                "type": "string",
+                "enum": ["first", "last", "min", "max"],
+            },
             "groupName": {"type": "string"},
             "valueName": {"type": "string"},
         },
diff --git a/frictionless/steps/table.py b/frictionless/steps/table.py
index 8e1ffdb1..659af4b9 100644
--- a/frictionless/steps/table.py
+++ b/frictionless/steps/table.py
@@ -222,10 +222,9 @@ class table_join(Step):
         *,
         resource=None,
         field_name=None,
-        use_hash=False,
-        mode="inner",
+        use_hash=None,
+        mode=None,
     ):
-        assert mode in ["inner", "left", "right", "outer", "cross", "negate"]
         self.setinitial("resource", resource)
         self.setinitial("fieldName", field_name)
         self.setinitial("useHash", use_hash)
@@ -238,8 +237,8 @@ class table_join(Step):
         target = resource
         source = self.get("resource")
         field_name = self.get("fieldName")
-        use_hash = self.get("useHash")
-        mode = self.get("mode")
+        use_hash = self.get("useHash", False)
+        mode = self.get("mode", "inner")
         if isinstance(source, str):
             source = target.package.get_resource(source)
         elif isinstance(source, dict):
@@ -276,7 +275,10 @@ class table_join(Step):
         "properties": {
             "resource": {},
             "fieldName": {"type": "string"},
-            "mode": {"type": "string"},
+            "mode": {
+                "type": "string",
+                "enum": ["inner", "left", "right", "outer", "cross", "negate"],
+            },
             "hash": {},
         },
     }
@@ -293,9 +295,8 @@ class table_melt(Step):
         *,
         variables=None,
         field_name=None,
-        to_field_names=["variable", "value"],
+        to_field_names=None,
     ):
-        assert len(to_field_names) == 2
         self.setinitial("variables", variables)
         self.setinitial("fieldName", field_name)
         self.setinitial("toFieldNames", to_field_names)
@@ -307,7 +308,7 @@ class table_melt(Step):
         table = resource.to_petl()
         variables = self.get("variables")
         field_name = self.get("fieldName")
-        to_field_names = self.get("toFieldNames")
+        to_field_names = self.get("toFieldNames", ["variable", "value"])
         field = resource.schema.get_field(field_name)
         resource.schema.fields.clear()
         resource.schema.add_field(field)
@@ -328,7 +329,7 @@ class table_melt(Step):
         "properties": {
             "fieldName": {"type": "string"},
             "variables": {"type": "array"},
-            "toFieldNames": {},
+            "toFieldNames": {"type": "array", "minItems": 2, "maxItems": 2},
         },
     }
 
@@ -490,9 +491,8 @@ class table_recast(Step):
         descriptor=None,
         *,
         field_name,
-        from_field_names=["variable", "value"],
+        from_field_names=None,
     ):
-        assert len(from_field_names) == 2
         self.setinitial("fieldName", field_name)
         self.setinitial("fromFieldNames", from_field_names)
         super().__init__(descriptor)
@@ -502,7 +502,7 @@ class table_recast(Step):
     def transform_resource(self, resource):
         table = resource.to_petl()
         field_name = self.get("fieldName")
-        from_field_names = self.get("fromFieldNames")
+        from_field_names = self.get("fromFieldNames", ["variable", "value"])
         resource.pop("schema", None)
         resource.data = table.recast(
             key=field_name,
@@ -518,7 +518,7 @@ class table_recast(Step):
         "required": ["fieldName"],
         "properties": {
             "fieldName": {"type": "string"},
-            "fromFieldNames": {},
+            "fromFieldNames": {"type": "array", "minItems": 2, "maxItems": 2},
         },
     }
 


Test patch:
diff --git a/tests/steps/test_row.py b/tests/steps/test_row.py
index 07e1b1d2..4ad65121 100644
--- a/tests/steps/test_row.py
+++ b/tests/steps/test_row.py
@@ -733,6 +733,28 @@ def test_step_row_sort_with_reverse():
     ]
 
 
+def test_step_row_sort_with_reverse_in_desriptor_issue_996():
+    source = Resource("data/transform.csv")
+    target = transform(
+        source,
+        steps=[
+            steps.row_sort({"fieldNames": ["id"], "reverse": True}),
+        ],
+    )
+    assert target.schema == {
+        "fields": [
+            {"name": "id", "type": "integer"},
+            {"name": "name", "type": "string"},
+            {"name": "population", "type": "integer"},
+        ]
+    }
+    assert target.read_rows() == [
+        {"id": 3, "name": "spain", "population": 47},
+        {"id": 2, "name": "france", "population": 66},
+        {"id": 1, "name": "germany", "population": 83},
+    ]
+
+
 # Split
 
 
@@ -782,6 +804,24 @@ def test_step_row_subset_conflicts():
     assert target.read_rows() == []
 
 
+def test_step_row_subset_conflicts_from_descriptor_issue_996():
+    source = Resource("data/transform.csv")
+    target = transform(
+        source,
+        steps=[
+            steps.row_subset({"subset": "conflicts", "fieldName": "id"}),
+        ],
+    )
+    assert target.schema == {
+        "fields": [
+            {"name": "id", "type": "integer"},
+            {"name": "name", "type": "string"},
+            {"name": "population", "type": "integer"},
+        ]
+    }
+    assert target.read_rows() == []
+
+
 def test_step_row_subset_conflicts_with_duplicates():
     source = Resource("data/transform.csv")
     target = transform(
diff --git a/tests/steps/test_table.py b/tests/steps/test_table.py
index b7f50b3d..98389af2 100644
--- a/tests/steps/test_table.py
+++ b/tests/steps/test_table.py
@@ -438,6 +438,33 @@ def test_step_table_join_mode_left():
     ]
 
 
+def test_step_table_join_mode_left_from_descriptor_issue_996():
+    source = Resource("data/transform.csv")
+    target = transform(
+        source,
+        steps=[
+            steps.table_normalize(),
+            steps.table_join(
+                {"fieldName": "id", "mode": "left"},
+                resource=Resource(data=[["id", "note"], [1, "beer"], [2, "vine"]]),
+            ),
+        ],
+    )
+    assert target.schema == {
+        "fields": [
+            {"name": "id", "type": "integer"},
+            {"name": "name", "type": "string"},
+            {"name": "population", "type": "integer"},
+            {"name": "note", "type": "string"},
+        ]
+    }
+    assert target.read_rows() == [
+        {"id": 1, "name": "germany", "population": 83, "note": "beer"},
+        {"id": 2, "name": "france", "population": 66, "note": "vine"},
+        {"id": 3, "name": "spain", "population": 47, "note": None},
+    ]
+
+
 def test_step_table_join_mode_right():
     source = Resource("data/transform.csv")
     target = transform(