From 7b068546114a2d8ab2499b0701525cc445283084 Mon Sep 17 00:00:00 2001 From: stefan <201365470+stefan1condeescu@users.noreply.github.com> Date: Tue, 28 Jul 2026 17:52:33 +0300 Subject: [PATCH] fix: avoid cell errors for header mismatches When the header has fewer or more columns than the schema, match the expected fields to the header. This avoids reporting missing-cell or extra-cell errors for every data row. Validation and analysis now only use fields that are actually available, while irregular rows still report cell errors as expected. Add regression tests for these cases. Refs #1791 --- frictionless/analyzer/analyzer.py | 17 ++-- .../package/__spec__/test_validate.py | 2 - .../resource/__spec__/test_validate_schema.py | 83 +++++++++++++++---- frictionless/resources/table.py | 20 +++-- frictionless/table/__spec__/test_header.py | 28 ++++++- frictionless/table/header.py | 23 ++++- 6 files changed, 143 insertions(+), 30 deletions(-) diff --git a/frictionless/analyzer/analyzer.py b/frictionless/analyzer/analyzer.py index f28c8ccdd6..f1948900f8 100644 --- a/frictionless/analyzer/analyzer.py +++ b/frictionless/analyzer/analyzer.py @@ -35,11 +35,18 @@ def analyze_table_resource( columns_data: Dict[str, List[Any]] = {} numeric = ["integer", "numeric", "number"] with resource: + expected_field_names = { + field.name for field in resource.header.get_expected_fields() + } + analysis_fields = [ + field + for field in resource.schema.fields + if field.name in expected_field_names + ] for row in resource.row_stream: null_columns = 0 - for field_name in row: - field = resource.schema.get_field(field_name) - cell = field.read_cell(row.get(field_name))[0] + for field in analysis_fields: + cell = field.read_cell(row.get(field.name))[0] if field.name not in columns_data: columns_data[field.name] = [] if cell is None: @@ -55,7 +62,7 @@ def analyze_table_resource( # Field/Column Stats if columns_data and detailed: analysis_report["correlations"] = {} - for field in resource.schema.fields: + for field in analysis_fields: analysis_report["fieldStats"][field.name] = {} if field.type not in analysis_report["variableTypes"]: @@ -89,7 +96,7 @@ def analyze_table_resource( ) # calculate correlation between variables(columns/fields) - for field_y in resource.schema.fields: + for field_y in analysis_fields: if field_y.type in numeric: # filter rows with nan values, correlation return nan if any of the # row has nan value. diff --git a/frictionless/package/__spec__/test_validate.py b/frictionless/package/__spec__/test_validate.py index c96ce4c301..80364da139 100644 --- a/frictionless/package/__spec__/test_validate.py +++ b/frictionless/package/__spec__/test_validate.py @@ -284,8 +284,6 @@ def test_package_validate_with_schema_issue_348(): report = package.validate() assert report.flatten(["rowNumber", "fieldNumber", "type"]) == [ [None, 4, "missing-label"], - [2, 4, "missing-cell"], - [3, 4, "missing-cell"], ] diff --git a/frictionless/resource/__spec__/test_validate_schema.py b/frictionless/resource/__spec__/test_validate_schema.py index d09527bb30..596bcbd5d3 100644 --- a/frictionless/resource/__spec__/test_validate_schema.py +++ b/frictionless/resource/__spec__/test_validate_schema.py @@ -43,8 +43,6 @@ def test_resource_validate_schema_extra_headers_and_cells(): report = resource.validate() assert report.flatten(["rowNumber", "fieldNumber", "type"]) == [ [None, 2, "extra-label"], - [2, 2, "extra-cell"], - [3, 2, "extra-cell"], ] extra_label_error = report.task.errors[0] @@ -368,10 +366,73 @@ def test_resource_validate_less_actual_fields_with_required_constraint_issue_950 print(report.flatten(["rowNumber", "fieldNumber", "type"])) assert report.flatten(["rowNumber", "fieldNumber", "type"]) == [ [None, 3, "missing-label"], - [2, 3, "constraint-error"], - [2, 3, "missing-cell"], - [3, 3, "constraint-error"], - [3, 3, "missing-cell"], + ] + + +def test_resource_validate_missing_label_preserves_irregular_row_issue_1791(): + data = [ + ["a", "b"], + ["1"], + ] + + schema = Schema.from_descriptor( + { + "fields": [ + {"name": "a"}, + {"name": "b"}, + {"name": "c"}, + ] + } + ) + + resource = TableResource(data=data, schema=schema) + report = resource.validate() + + assert report.flatten(["rowNumber", "fieldNumber", "fieldName", "type"]) == [ + [None, 3, "c", "missing-label"], + [2, 2, "b", "missing-cell"], + ] + + +@pytest.mark.parametrize( + "schema_descriptor", + [ + { + "fields": [ + {"name": "id"}, + {"name": "missing", "constraints": {"unique": True}}, + ] + }, + { + "fields": [{"name": "id"}, {"name": "missing"}], + "primaryKey": "missing", + }, + { + "fields": [{"name": "id"}, {"name": "missing"}], + "foreignKeys": [ + { + "fields": "missing", + "reference": {"resource": "", "fields": "id"}, + } + ], + }, + ], + ids=["unique", "primary-key", "foreign-key"], +) +def test_resource_validate_missing_label_skips_integrity_checks_issue_1791( + schema_descriptor, +): + data = [["id"], ["1"], ["2"]] + schema = Schema.from_descriptor(schema_descriptor) + resource = TableResource( + data=data, + schema=schema, + dialect=Dialect(header_case=False), + ) + report = resource.validate() + + assert report.flatten(["rowNumber", "fieldNumber", "fieldName", "type"]) == [ + [None, 2, "missing", "missing-label"], ] @@ -543,7 +604,7 @@ def test_resource_validate_fields_match_reordered_labels(fields_match, expected) @pytest.mark.parametrize( "fields_match, expected", [ - ("exact", [[None, 2, "", "extra-label"], [2, 2, "", "extra-cell"]]), + ("exact", [[None, 2, "", "extra-label"]]), ("equal", [[None, 2, "", "extra-label"]]), ("superset", [[None, 2, "", "extra-label"]]), ("subset", []), @@ -558,13 +619,7 @@ def test_resource_validate_fields_match_extra_label(fields_match, expected): @pytest.mark.parametrize( "fields_match, expected", [ - ( - "exact", - [ - [None, 3, "extra", "missing-label"], - [2, 3, "extra", "missing-cell"], - ], - ), + ("exact", [[None, 3, "extra", "missing-label"]]), ("equal", [[None, 3, "extra", "missing-label"]]), ("subset", [[None, 3, "extra", "missing-label"]]), ("superset", []), diff --git a/frictionless/resources/table.py b/frictionless/resources/table.py index 306b360767..d830fda826 100644 --- a/frictionless/resources/table.py +++ b/frictionless/resources/table.py @@ -285,22 +285,32 @@ def __open_row_stream(self): # those fields, so build them once here and reuse them for every row. expected_fields: List[Field] = self.header.get_expected_fields() handlers = create_cell_handlers(expected_fields) + expected_field_names = {field.name for field in expected_fields} + + primary_key_fields = set(self.schema.primary_key) + has_primary_key = bool(primary_key_fields) and primary_key_fields.issubset( + expected_field_names + ) - # Create state memory_unique: Dict[str, Any] = {} memory_primary: Dict[Tuple[Any], Any] = {} foreign_groups: List[Any] = [] - is_integrity = bool(self.schema.primary_key) - for field in self.schema.fields: + is_integrity = has_primary_key + + for field in expected_fields: if field.constraints.get("unique"): memory_unique[field.name] = {} is_integrity = True + if self.__lookup: for fk in self.schema.foreign_keys: + target_key = tuple(fk["fields"]) + if not set(target_key).issubset(expected_field_names): + continue group = {} group["sourceName"] = fk["reference"]["resource"] group["sourceKey"] = tuple(fk["reference"]["fields"]) - group["targetKey"] = tuple(fk["fields"]) + group["targetKey"] = target_key foreign_groups.append(group) is_integrity = True @@ -335,7 +345,7 @@ def row_stream(): row.errors.append(error) # Primary Key Error - if is_integrity and self.schema.primary_key: + if has_primary_key: try: cells = self.primary_key_cells(row, self.dialect.header_case) except KeyError: diff --git a/frictionless/table/__spec__/test_header.py b/frictionless/table/__spec__/test_header.py index 342e6a7e2b..336a68c579 100644 --- a/frictionless/table/__spec__/test_header.py +++ b/frictionless/table/__spec__/test_header.py @@ -83,8 +83,24 @@ def _make_header(labels, field_names, *, fields_match="exact", ignore_case=False ["a"], "exact", False, + ["a", "extra"], + id="exact: extra labels get a default any-typed field", + ), + pytest.param( + ["a"], + ["a", "b"], + "exact", + False, + ["a"], + id="exact: fields are truncated to match labels", + ), + pytest.param( + [], ["a"], - id="exact: extra labels get no field", + "exact", + False, + ["a"], + id="exact: a missing header keeps the schema fields", ), *[ pytest.param( @@ -139,13 +155,21 @@ def test_get_expected_fields( assert actual == expected_names -@pytest.mark.parametrize("fields_match", NAME_MATCHED) +@pytest.mark.parametrize("fields_match", ["exact", *NAME_MATCHED]) def test_get_expected_fields_default_field_is_any_typed(fields_match): header = _make_header(["a", "extra"], ["a"], fields_match=fields_match) expected = header.get_expected_fields() assert expected[1].type == "any" +def test_get_expected_fields_exact_uses_unique_names_for_extra_fields(): + header = _make_header(["a", "a"], ["a"], fields_match="exact") + expected = header.get_expected_fields() + + assert len(expected) == 2 + assert len({field.name for field in expected}) == 2 + + @pytest.mark.parametrize("fields_match", NAME_MATCHED) def test_get_expected_fields_raises_on_duplicate_labels(fields_match): header = _make_header(["a", "a"], ["a"], fields_match=fields_match) diff --git a/frictionless/table/header.py b/frictionless/table/header.py index 0b3477e748..ba731bceae 100644 --- a/frictionless/table/header.py +++ b/frictionless/table/header.py @@ -140,7 +140,8 @@ def __matches_by_name(self) -> bool: def get_expected_fields(self) -> List[Field]: """Returns the fields, in the order expected in the data. - Under `exact`, this is just the schema fields unchanged. + Under `exact`, schema fields keep their order and are truncated or + extended with `any`-typed fields to match the labels. Under the name-matched modes, fields are reordered to match the labels; labels without a matching field get a fresh `any`-typed field (even @@ -151,10 +152,28 @@ def get_expected_fields(self) -> List[Field]: if self.__expected_fields is not None: return self.__expected_fields - if not self.__matches_by_name: + if self.missing: self.__expected_fields = self.__fields return self.__expected_fields + if not self.__matches_by_name: + expected = self.__fields[: len(self.__labels)] + used_names = {field.name for field in expected} + + for label in self.__labels[len(expected) :]: + name = label + suffix = 2 + + while name in used_names: + name = f"{label}{suffix}" + suffix += 1 + + used_names.add(name) + expected.append(Field.from_descriptor({"name": name, "type": "any"})) + + self.__expected_fields = expected + return self.__expected_fields + # ignore_case can make fields ambiguous as their keys are identical, # e.g. "A" and "a" for group in self.__matching.ambiguous_fields: