Skip to content

fix: 🐛 filter out path and data errors correctly #134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 15, 2025
Merged

fix: 🐛 filter out path and data errors correctly #134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/check_datapackage/check.py
View file
Open in desktop
Copy link
Contributor Author

@martonvago martonvago Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What this function should do:

  • only remove the parent error if there are other errors in the group
  • if both data and path are missing on the resource, unite those two errors into a single better error
  • remove all data / path required errors (whether there are two errors or just one)

Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,15 @@ def _handle_S_resources_x(
) -> list[SchemaError]:
"""Do not flag missing `path` and `data` separately."""
errors_in_group = _filter(schema_errors, lambda error: error.parent == parent_error)
# If the parent error is caused by other errors, remove it
if errors_in_group:
schema_errors.remove(parent_error)

path_or_data_required_errors = _filter(
errors_in_group, _path_or_data_required_error
)

# If path and data are both missing, add a more informative error
if path_or_data_required_errors:
schema_errors.remove(parent_error)
if len(path_or_data_required_errors) > 1:
schema_errors.append(
SchemaError(
message=(
Expand All @@ -162,7 +164,7 @@ def _handle_S_resources_x(
)
)

# Remove all original required errors on $.resources[x].path and $.resources[x].data
# Remove all required errors on path and data
return _filter(
schema_errors, lambda error: error not in path_or_data_required_errors
)
Expand Down
11 changes: 11 additions & 0 deletions tests/test_check.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,17 @@ def test_fail_with_resource_name_path_and_data_missing():
assert issues[1].type == "required"


def test_fail_with_only_resource_name_missing():
descriptor = example_package_descriptor()
del descriptor["resources"][0]["name"]

issues = check(descriptor)

assert len(issues) == 1
assert issues[0].jsonpath == "$.resources[0].name"
assert issues[0].type == "required"


Comment on lines +194 to +204
Copy link
Contributor Author

@martonvago martonvago Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was failing because the name missing makes jsonschema check against both the path-based and data-based schemas, resulting in a "data is missing" issue.

But we know that, because the path is present here, we should only be checking against the path-based schema.

def test_fail_with_multiple_resources():
descriptor = example_package_descriptor()
descriptor["resources"].append(example_resource_descriptor())
Expand Down