diff --git a/frictionless/resource/__spec__/test_validate_schema.py b/frictionless/resource/__spec__/test_validate_schema.py index f63904f272..c7bb612aaf 100644 --- a/frictionless/resource/__spec__/test_validate_schema.py +++ b/frictionless/resource/__spec__/test_validate_schema.py @@ -311,6 +311,28 @@ def test_resource_validate_resource_duplicate_labels_with_sync_schema_issue_910( ] +def test_resource_validate_duplicate_labels_ignoring_header_case(): + schema = Schema.from_descriptor( + { + "fields": [{"name": "name", "type": "string"}], + "fieldsMatch": "partial", + } + ) + resource = TableResource( + [["Name", "name"], ["a", "b"]], + schema=schema, + dialect=Dialect(header_case=False), + ) + report = resource.validate() + assert report.flatten(["type", "note"]) == [ + [ + "error", + 'matching fields by name ("fieldsMatch": "partial") ' + "requires unique labels in the header", + ], + ] + + def test_resource_validate_less_actual_fields_with_required_constraint_issue_950(): schema = Schema.describe("data/table.csv") schema.add_field(fields.AnyField(name="bad", constraints={"required": True})) diff --git a/frictionless/table/header.py b/frictionless/table/header.py index ce7907b025..229df3ed4e 100644 --- a/frictionless/table/header.py +++ b/frictionless/table/header.py @@ -6,6 +6,7 @@ from .. import errors, helpers, types from ..exception import FrictionlessException from ..schema import Field +from .label_matching import LabelMatching # The `fieldsMatch` modes are told apart by which mismatch they tolerate: a # label with no matching field, or a declared field with no matching label @@ -18,12 +19,15 @@ class Header(List[str]): # type: ignore """Header representation + Compares the header row read from the data source (the "labels") with the + fields declared in the schema, and reports the mismatches as errors. + > Constructor of this object is not Public API Parameters: - labels (any[]): header row labels - fields (Field[]): table fields - row_numbers (int[]): row numbers + labels (any[]): the header row as read from the data source + fields (Field[]): the fields declared in the schema, in schema order + row_numbers (int[]): row numbers the header spans in the data source ignore_case (bool): ignore case fields_match (str): how the fields match the data source @@ -54,13 +58,14 @@ def __init__( self.__labels = labels self.__errors: List[errors.HeaderError] = [] self.__expected_fields: Optional[List[Field]] = None + self.__matching = LabelMatching(labels, self.__fields, ignore_case=ignore_case) self.__process() @cached_property def labels(self): """ Returns: - Schema: table labels + str[]: the header row as read from the data source """ return self.__labels @@ -68,7 +73,7 @@ def labels(self): def fields(self): """ Returns: - Schema: table fields + Field[]: copies of the schema fields, in schema order """ return self.__fields @@ -76,7 +81,7 @@ def fields(self): def field_names(self): """ Returns: - str[]: table field names + str[]: the names of the schema fields, in schema order """ return self.__field_names @@ -150,7 +155,7 @@ def get_expected_fields(self) -> List[Field]: self.__expected_fields = self.__fields return self.__expected_fields - if len(self.__labels) != len(set(self.__labels)): + if self.__matching.has_duplicate_labels: note = ( f'matching fields by name ("fieldsMatch": "{self.__fields_match}") ' "requires unique labels in the header" @@ -159,7 +164,7 @@ def get_expected_fields(self) -> List[Field]: expected: List[Field] = [] for label in self.__labels: - field = self.__find_field_by_name(label) + field = self.__matching.matching_field(label) if field is None: field = Field.from_descriptor({"name": label, "type": "any"}) expected.append(field) @@ -191,7 +196,7 @@ def _get_extra_labels(self) -> List[Tuple[int, str]]: return [ (number, label) for number, label in enumerate(labels, start=1) - if self.__find_field_by_name(label) is None + if self.__matching.matching_field(label) is None ] def _get_missing_fields(self) -> List[Tuple[int, Field]]: @@ -215,39 +220,19 @@ def _get_missing_fields(self) -> List[Tuple[int, Field]]: if not self.__matches_by_name: missing = fields[len(labels) :] if len(fields) > len(labels) else [] else: - normalized_labels = [self.__normalize(label) for label in labels] - - def is_absent(field: Field) -> bool: - return self.__normalize(field.name) not in normalized_labels def is_required(field: Field) -> bool: return field.required or ( field.schema is not None and field.name in field.schema.primary_key ) - missing = [field for field in fields if is_absent(field)] + missing = self.__matching.unmatched_fields if self.__fields_match in TOLERATES_MISSING_FIELDS: missing = [field for field in missing if is_required(field)] start = len(labels) + 1 return [(start + offset, field) for offset, field in enumerate(missing)] - def __has_matching_field(self) -> bool: - """Whether at least one label corresponds to a schema field""" - return any( - self.__find_field_by_name(label) is not None for label in self.__labels - ) - - def __find_field_by_name(self, name: str) -> Optional[Field]: - target = self.__normalize(name) - for f in self.__fields: - if self.__normalize(f.name) == target: - return f - return None - - def __normalize(self, s: str) -> str: - return s.lower() if self.__ignore_case else s - # Convert def to_str(self): @@ -288,11 +273,7 @@ def __process(self): ) # Unmatched header - if ( - self.__fields_match == "partial" - and fields - and not self.__has_matching_field() - ): + if self.__fields_match == "partial" and fields and not self.__matching.has_match: self.__errors.append( errors.UnmatchedHeaderError( note="", diff --git a/frictionless/table/label_matching.py b/frictionless/table/label_matching.py new file mode 100644 index 0000000000..b03bb9613f --- /dev/null +++ b/frictionless/table/label_matching.py @@ -0,0 +1,68 @@ +from __future__ import annotations + +from typing import Dict, List, Optional + +from ..schema import Field + + +class LabelMatching: + """Pairs the labels read from the data source with the schema fields, by name. + + Parameters: + labels (str[]): the header row as read from the data source + fields (Field[]): the fields declared in the schema, in schema order + ignore_case (bool): compare labels and field names case-insensitively + """ + + def __init__( + self, + labels: List[str], + fields: List[Field], + *, + ignore_case: bool = False, + ) -> None: + self.__labels = labels + self.__fields = fields + self.__ignore_case = ignore_case + + # Keyed by normalized name, in schema order; the first field wins in + # case of duplicates under normalization, so the duplicate fields are + # lost + fields_by_key: Dict[str, Field] = {} + for field in fields: + fields_by_key.setdefault(self.__normalize(field.name), field) + + self.__fields_by_key = fields_by_key + + def matching_field(self, label: str) -> Optional[Field]: + """Returns the field the given label matches, or None if there is none""" + return self.__fields_by_key.get(self.__normalize(label)) + + @property + def unmatched_fields(self) -> List[Field]: + """The fields no label matches, in schema order""" + matched = {self.__normalize(label) for label in self.__labels} + return [ + field + for field in self.__fields + if self.__normalize(field.name) not in matched + ] + + @property + def has_duplicate_labels(self) -> bool: + """Whether two labels match the same field, which makes the mapping ambiguous + + Labels are compared the way they are matched, so under `ignore_case` + two labels differing only by case are duplicates. + """ + keys = [self.__normalize(label) for label in self.__labels] + return len(keys) != len(set(keys)) + + @property + def has_match(self) -> bool: + """Whether at least one label matches a schema field""" + return any(self.matching_field(label) is not None for label in self.__labels) + + def __normalize(self, name: str) -> str: + """The normalized value a label and a field name are compared through""" + return name.lower() if self.__ignore_case else name