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: