Remove schema mutation for schema-sync *sigh*

Author: pierrecamilleri

frictionless/detector/detector.py

@@ -411,33 +411,6 @@ def detect_schema(
                     fields[index] = AnyField(name=name, schema=schema)  # type: ignore
             schema.fields = fields  # type: ignore
 
-        # Sync schema
-        if self.schema_sync:
-            if labels:
-                case_sensitive = options["header_case"]
-
-                if not case_sensitive:
-                    labels = [label.lower() for label in labels]
-
-                if len(labels) != len(set(labels)):
-                    note = '"schema_sync" requires unique labels in the header'
-                    raise FrictionlessException(note)
-
-                mapped_fields = self.mapped_schema_fields_names(
-                    schema.fields,  # type: ignore
-                    case_sensitive,
-                )
-
-                self.rearrange_schema_fields_given_labels(
-                    mapped_fields,
-                    schema,
-                    labels,
-                )
-
-                self.add_missing_required_labels_to_schema_fields(
-                    mapped_fields, schema, labels, case_sensitive
-                )
-
         # Patch schema
         if self.schema_patch:
             patch = deepcopy(self.schema_patch)
@@ -452,56 +425,3 @@ def detect_schema(
 
         return schema
 
-    @staticmethod
-    def mapped_schema_fields_names(
-        fields: List[Field], case_sensitive: bool
-    ) -> Dict[str, Field]:
-        """Create a dictionnary to map field names with schema fields"""
-        if case_sensitive:
-            return {field.name: field for field in fields}
-        else:
-            return {field.name.lower(): field for field in fields}
-
-    @staticmethod
-    def rearrange_schema_fields_given_labels(
-        fields_mapping: Dict[str, Field],
-        schema: Schema,
-        labels: List[str],
-    ):
-        """Rearrange fields according to the order of labels. All fields
-        missing from labels are dropped"""
-        schema.clear_fields()
-
-        for name in labels:
-            default_field = Field.from_descriptor({"name": name, "type": "any"})
-            field = fields_mapping.get(name, default_field)
-            schema.add_field(field)
-
-    def add_missing_required_labels_to_schema_fields(
-        self,
-        fields_mapping: Dict[str, Field],
-        schema: Schema,
-        labels: List[str],
-        case_sensitive: bool,
-    ):
-        """This method aims to add missing required labels and
-        primary key field not in labels to schema fields.
-        """
-        for name, field in fields_mapping.items():
-            if (
-                self.field_is_required(field, schema, case_sensitive)
-                and name not in labels
-            ):
-                schema.add_field(field)
-
-    @staticmethod
-    def field_is_required(
-        field: Field,
-        schema: Schema,
-        case_sensitive: bool,
-    ) -> bool:
-        if case_sensitive:
-            return field.required or field.name in schema.primary_key
-        else:
-            lower_primary_key = [pk.lower() for pk in schema.primary_key]
-            return field.required or field.name.lower() in lower_primary_key

frictionless/resource/__spec__/test_validate.py

@@ -509,10 +509,12 @@ def test_resource_validate_detector_sync_schema():
     )
     report = resource.validate()
     assert report.valid
+    # schema_sync no longer mutates the user-provided schema: the order
+    # given by the user is preserved.
     assert resource.schema.to_descriptor() == {
         "fields": [
-            {"name": "name", "type": "string"},
             {"name": "id", "type": "integer"},
+            {"name": "name", "type": "string"},
         ],
     }
 

frictionless/table/header.py

@@ -1,7 +1,7 @@
 from __future__ import annotations
 
 from functools import cached_property
-from typing import List, Optional
+from typing import List, Optional, Tuple
 
 from ..exception import FrictionlessException
 from ..schema import Field
@@ -155,31 +155,42 @@ def _get_extra_labels(self) -> List[str]:
                 return self.__labels[len(self.__fields) :]
         return []
 
-    def _get_missing_fields(self) -> List[Field]:
-        """Returns schema fields that don't have a corresponding label.
+    def _get_missing_fields(self) -> List[Tuple[int, Field]]:
+        """Returns (field_number, field) pairs for schema fields that don't
+        have a corresponding label.
 
         Without `schema_sync`, fields beyond the labels count are considered
         missing. With `schema_sync`, only required fields whose name is not
         among the labels are missing.
+
+        The field_number is `len(labels) + offset + 1` in both modes: under
+        no-sync the missing fields are precisely the tail of the schema, so
+        this matches their position; under sync the missing fields have no
+        natural position in the data, so we place them after the labels by
+        convention.
         """
         fields = self.__fields
         labels = self.__labels
-        if not self.__schema_sync:
-            if len(fields) > len(labels):
-                return fields[len(labels) :]
-            return []
 
-        normalized_labels = [self.__normalize(label) for label in labels]
+        if not self.__schema_sync:
+            missing = fields[len(labels) :] if len(fields) > len(labels) else []
+        else:
+            normalized_labels = [self.__normalize(label) for label in labels]
+
+            def required_and_missing(field: Field) -> bool:
+                required = field.required or (
+                    field.schema is not None
+                    and field.name in field.schema.primary_key
+                )
+                return (
+                    required
+                    and self.__normalize(field.name) not in normalized_labels
+                )
 
-        def required_and_missing(field: Field) -> bool:
-            required = field.required or (
-                field.schema is not None and field.name in field.schema.primary_key
-            )
-            return (
-                required and self.__normalize(field.name) not in normalized_labels
-            )
+            missing = [field for field in fields if required_and_missing(field)]
 
-        return [field for field in fields if required_and_missing(field)]
+        start = len(labels) + 1
+        return [(start + offset, field) for offset, field in enumerate(missing)]
 
     def __find_field_by_name(self, name: str) -> Optional[Field]:
         target = self.__normalize(name)
@@ -232,24 +243,28 @@ def __process(self):
             )
 
         # Missing fields
-        missing_fields = self._get_missing_fields()
-        if missing_fields:
-            missing_ids = {id(field) for field in missing_fields}
-            for field_number, field in enumerate(fields, start=1):
-                if field is None or id(field) not in missing_ids:
-                    continue
-                self.__errors.append(
-                    errors.MissingLabelError(
-                        note="",
-                        labels=list(map(str, labels)),
-                        row_numbers=self.__row_numbers,
-                        label="",
-                        field_name=field.name,
-                        field_number=field_number,
-                    )
+        for field_number, field in self._get_missing_fields():
+            self.__errors.append(
+                errors.MissingLabelError(
+                    note="",
+                    labels=list(map(str, labels)),
+                    row_numbers=self.__row_numbers,
+                    label="",
+                    field_name=field.name,
+                    field_number=field_number,
                 )
+            )
 
         # Iterate items
+        # Under schema_sync, labels and fields are matched by name (not by
+        # position), so the positional comparisons below (blank label vs
+        # field at the same index, incorrect label vs field name at the same
+        # index) don't apply. Duplicate labels are still invalid, but they
+        # are rejected earlier by get_expected_fields(), which raises a
+        # FrictionlessException — so detecting them here would be redundant.
+        if self.__schema_sync:
+            return
+
         field_number = 0
         for field, label in zip(fields, labels):
             field_number += 1