Skip to content
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.

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

feat: ✨ add check_properties() #967

Merged
merged 85 commits into from
Jan 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
85 commits
Select commit Hold shift + click to select a range
55c03c6
refactor: :recycle: use custom CheckError in checks
martonvago Dec 18, 2024
958bb18
feat: :sparkles: improve CheckError
martonvago Dec 19, 2024
a619ded
test: :white_check_mark: add tests for helper functions
martonvago Dec 19, 2024
3586c51
refactor: :recycle: return early from function
martonvago Dec 19, 2024
d3de146
docs: :memo: update test docstrings
martonvago Dec 19, 2024
05e7d41
apply suggestions from code review
martonvago Jan 10, 2025
52d3a24
chore(pre-commit): :pencil2: automatic fixes
pre-commit-ci[bot] Jan 10, 2025
64d7a87
Merge branch 'main' into feat/check-error
martonvago Jan 10, 2025
8196dc0
feat: :sparkles: put required fields into constants
martonvago Jan 10, 2025
8c701d8
feat: :sparkles: add simple helper functions
martonvago Jan 10, 2025
b6a5a86
feat: :sparkles: add functions to check required and blank fields
martonvago Jan 10, 2025
e9da123
feat: :sparkles: add functions for collecting Sprout-specific package…
martonvago Jan 10, 2025
6133ac9
feat: :sparkles: add check_resource_properties
martonvago Jan 10, 2025
b179dbe
feat: :sparkles: add check_package_properties
martonvago Jan 10, 2025
166958b
feat: :sparkles: add check_properties
martonvago Jan 10, 2025
dabbb75
docs: :memo: fix docstring
martonvago Jan 10, 2025
a08ce8b
apply suggestions from code review
martonvago Jan 13, 2025
1dd1e1e
refactor: :recycle: rename function to validation_errors_to_check_errors
martonvago Jan 13, 2025
c595aa9
refactor: :recycle: rename file to validation_errors_to_check_errors
martonvago Jan 13, 2025
cb49bd1
docs: :memo: add more detail to docstring
martonvago Jan 13, 2025
d603d82
refactor: :recycle: rename constant to PACKAGE_RECOMMENDED_FIELDS
martonvago Jan 14, 2025
abfc4c5
fix: :bug: include `data` in resource required fields
martonvago Jan 14, 2025
6611019
refactor: :recycle: make structure of PACKAGE_SPROUT_REQUIRED_FIELDS …
martonvago Jan 14, 2025
fea4a5f
refactor: :recycle: drop fields using pop
martonvago Jan 14, 2025
1cbec8a
apply suggestions from code review
martonvago Jan 14, 2025
c5b784f
refactor: :recycle: rename function to get_sprout_specific_resource_e…
martonvago Jan 14, 2025
c854a2d
refactor: :recycle: rename file to get_sprout_specific_resource_errors
martonvago Jan 14, 2025
ee5edf0
docs: :memo: update test names and docstrings
martonvago Jan 14, 2025
ee05805
Merge branch 'feat/check-error' into feat/sprout-checks-1-required-fi…
martonvago Jan 14, 2025
e43d13e
Merge branch 'feat/sprout-checks-1-required-fields' into feat/sprout-…
martonvago Jan 14, 2025
b174133
Merge branch 'feat/sprout-checks-2-simple-helper-functions' into feat…
martonvago Jan 14, 2025
562f93b
Merge branch 'feat/sprout-checks-3-required-and-blank-checks' into fe…
martonvago Jan 14, 2025
2391eed
Merge branch 'feat/sprout-checks-4-sprout-specific-package-and-resour…
martonvago Jan 14, 2025
569b947
refactor: :recycle: rename function to exclude_non_sprout_resource_er…
martonvago Jan 14, 2025
87418fb
refactor: :recycle: rename file to exclude_non_sprout_resource_errors
martonvago Jan 14, 2025
72db9d8
Merge branch 'feat/sprout-checks-2-simple-helper-functions' into feat…
martonvago Jan 14, 2025
99c783d
Merge branch 'feat/sprout-checks-3-required-and-blank-checks' into fe…
martonvago Jan 14, 2025
56d4cee
Merge branch 'feat/sprout-checks-4-sprout-specific-package-and-resour…
martonvago Jan 14, 2025
8bbdd4d
refactor: :recycle: correct function name after rename
martonvago Jan 14, 2025
accee38
Merge branch 'feat/sprout-checks-5-check-resource-properties' into fe…
martonvago Jan 14, 2025
0df3b4d
Merge branch 'feat/sprout-checks-6-check-package-properties' into fea…
martonvago Jan 14, 2025
9270df6
refactor: :recycle: correct function name after rename
martonvago Jan 14, 2025
144b12f
apply suggestions from code review
martonvago Jan 15, 2025
0e3a772
refactor: :recycle: update test names and docstrings
martonvago Jan 15, 2025
9e49ebb
apply suggestions from code review
martonvago Jan 15, 2025
cad7d43
docs: :memo: update test names and docstrings
martonvago Jan 17, 2025
f017c19
Merge branch 'feat/sprout-checks-3-required-and-blank-checks' into fe…
martonvago Jan 17, 2025
33d0062
Merge branch 'feat/sprout-checks-4-sprout-specific-package-and-resour…
martonvago Jan 17, 2025
6c90df7
refactor: :recycle: use ExceptionGroup instead of custom summary error
martonvago Jan 17, 2025
0a00856
docs: :memo: update test names and docstrings
martonvago Jan 17, 2025
02abdb5
docs: :memo: update test names and docstrings
martonvago Jan 17, 2025
a052837
apply suggestions from code review
martonvago Jan 17, 2025
5c1735a
Merge branch 'feat/sprout-checks-5-check-resource-properties' into fe…
martonvago Jan 17, 2025
7bfe5e6
docs: :memo: update test names and docstrings
martonvago Jan 17, 2025
e86e2c3
Merge branch 'feat/sprout-checks-5-check-resource-properties' into fe…
martonvago Jan 17, 2025
7bb74db
refactor: :recycle: use ExceptionGroup instead of custom summary error
martonvago Jan 17, 2025
eed70aa
docs: :memo: update test names and docstrings
martonvago Jan 17, 2025
42e40b6
apply suggestions from code review
martonvago Jan 17, 2025
4771b86
Merge branch 'feat/sprout-checks-6-check-package-properties' into fea…
martonvago Jan 17, 2025
4d0b812
refactor: :recycle: use ExceptionGroup instead of custom summary error
martonvago Jan 17, 2025
5350b2e
docs: :memo: update test names and docstrings
martonvago Jan 17, 2025
b33c387
Merge branch 'main' into feat/sprout-checks-2-simple-helper-functions
lwjohnst86 Jan 20, 2025
f0510d8
Merge branch 'main' into feat/sprout-checks-2-simple-helper-functions
lwjohnst86 Jan 20, 2025
7c19388
refactor: :recycle: rename functions
martonvago Jan 20, 2025
abc47e9
refactor: :recycle: rename files
martonvago Jan 20, 2025
a0d4897
refactor: :recycle: update error message and assertions
martonvago Jan 20, 2025
fd6282c
refactor: :recycle: pull out type error message into constant
martonvago Jan 20, 2025
7881d41
Merge branch 'feat/sprout-checks-2-simple-helper-functions' into feat…
martonvago Jan 20, 2025
f531760
refactor: :recycle: pull out error messages into constants
martonvago Jan 20, 2025
e5cefa8
refactor: :recycle: rename tests
martonvago Jan 20, 2025
cc9f4ba
Merge branch 'feat/sprout-checks-3-required-and-blank-checks' into fe…
martonvago Jan 20, 2025
8e6a91e
refactor: :recycle: fix imports after rename
martonvago Jan 20, 2025
9cbff8b
Merge branch 'feat/sprout-checks-4-sprout-specific-package-and-resour…
martonvago Jan 20, 2025
4039200
apply suggestions from code review
martonvago Jan 20, 2025
62ab91f
style: :art: fix linter
martonvago Jan 20, 2025
b8c7d1c
test: :white_check_mark: don't check error message in test
martonvago Jan 20, 2025
54e3a6c
Merge branch 'feat/sprout-checks-5-check-resource-properties' into fe…
martonvago Jan 20, 2025
cdbc133
apply suggestions from code review
martonvago Jan 20, 2025
845d9f2
style: :art: fix linter
martonvago Jan 20, 2025
2cfd170
Merge branch 'feat/sprout-checks-6-check-package-properties' into fea…
martonvago Jan 20, 2025
e06afa4
apply suggestions from code review
martonvago Jan 20, 2025
14d5376
Merge branch 'main' into feat/sprout-checks-7-check-properties
lwjohnst86 Jan 21, 2025
7335c8b
docs: :memo: update docstrings
martonvago Jan 21, 2025
feae835
revert: :rewind: commitizen seems to have updating other package vers…
lwjohnst86 Jan 21, 2025
4b1b833
Merge branch 'feat/sprout-checks-7-check-properties' of https://githu…
lwjohnst86 Jan 21, 2025
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
4 changes: 2 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,14 @@ pytest = "^8.3.2"
time-machine = "^2.15.0"

[tool.poetry.group.dev.dependencies]
ruff = "^0.10.0"
ruff = "^0.9.0"
datamodel-code-generator = {extras = ["http"], version = "^0.26.0"}
commitizen = "^4.0.0"
pre-commit = "^4.0.1"

[tool.poetry.group.docs.dependencies]
jupyter = "^1.1.1"
quartodoc = "^0.10.0"
quartodoc = "^0.9.0"

[build-system]
requires = ["poetry-core"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def check_package_properties(properties: dict, check_required=True) -> dict:
`properties` if all checks pass.

Raises:
ExceptionGroup: A group of `CheckError`s, one for each check that failed.
ExceptionGroup: A group of `CheckError`s, one error per failed check.
"""
errors = checks.check_package_properties(properties)

Expand Down
48 changes: 48 additions & 0 deletions seedcase_sprout/core/sprout_checks/check_properties.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
from seedcase_sprout.core import checks
from seedcase_sprout.core.sprout_checks.exclude_non_sprout_resource_errors import (
exclude_non_sprout_resource_errors,
)
from seedcase_sprout.core.sprout_checks.get_sprout_package_errors import (
get_sprout_package_errors,
)
from seedcase_sprout.core.sprout_checks.get_sprout_resource_errors import (
get_sprout_resource_errors,
)


def check_properties(properties: dict, check_required=True) -> dict:
"""Checks that `properties` matches requirements in Sprout.

`properties` is checked against the Data Package standard and Sprout-specific
requirements. Both package and resource properties are checked.

Args:
properties: The full package properties to check, including resource properties.
check_required: Whether the function should enforce the presence of required
fields. Defaults to True.

Returns:
`properties`, if all checks pass.

Raises:
ExceptionGroup: A group of `CheckError`s, one error per failed check.
"""
errors = checks.check_properties(properties)
errors = exclude_non_sprout_resource_errors(errors)

if not check_required:
errors = [error for error in errors if error.validator != "required"]

errors += get_sprout_package_errors(properties, check_required)

for index, resource in enumerate(properties.get("resources", [])):
errors += get_sprout_resource_errors(resource, check_required, index)

errors = sorted(set(errors))

if errors:
raise ExceptionGroup(
f"The follow checks failed on the properties:\n{properties}", errors
)

return properties
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def check_resource_properties(properties: dict, check_required: bool = True) ->
`properties`, if all checks passed.

Raises:
ExceptionGroup: A group of `CheckError`s, one for each check that failed.
ExceptionGroup: A group of `CheckError`s, one error per failed check.
"""
errors = checks.check_resource_properties(properties)
errors = exclude_non_sprout_resource_errors(errors)
Expand Down
171 changes: 171 additions & 0 deletions tests/core/sprout_checks/test_check_properties_sprout.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
from pathlib import Path

from pytest import fixture, mark, raises

from seedcase_sprout.core.checks.check_error import CheckError
from seedcase_sprout.core.properties import PackageProperties, ResourceProperties
from seedcase_sprout.core.sprout_checks.check_properties import check_properties
from seedcase_sprout.core.sprout_checks.get_blank_value_for_type import (
get_blank_value_for_type,
)
from seedcase_sprout.core.sprout_checks.required_fields import (
PACKAGE_SPROUT_REQUIRED_FIELDS,
RESOURCE_SPROUT_REQUIRED_FIELDS,
)


@fixture
def properties():
return PackageProperties(
name="package-1",
id="abc1",
title="Package 1",
description="A package.",
version="1.0.0",
created="2024-05-14T05:00:01+00:00",
licenses=[{"name": "a-license"}],
contributors=[{"title": "a contributor"}],
sources=[{"title": "a source"}],
resources=[
ResourceProperties(
name="resource-1",
path=str(Path("resources", "1", "data.parquet")),
title="Resource 1",
description="A resource.",
),
ResourceProperties(
name="resource-2",
path=str(Path("resources", "2", "data.parquet")),
title="Resource 2",
description="A second resource.",
),
],
).compact_dict


@mark.parametrize("check_required", [True, False])
def test_check_passes_full_properties(properties, check_required):
"""Should pass if all required fields are present and correct."""
assert check_properties(properties, check_required=check_required) == properties


def test_passes_partial_properties_without_required_check():
"""Should pass properties with missing required fields when these are not
enforced."""
assert check_properties({}, check_required=False) == {}


@mark.parametrize("field", [*PACKAGE_SPROUT_REQUIRED_FIELDS.keys(), "resources"])
def test_raises_error_if_package_required_field_is_missing(properties, field):
"""Should raise an error if a required field is missing among the package
properties."""
del properties[field]

with raises(ExceptionGroup) as error_info:
check_properties(properties, check_required=True)

errors = error_info.value.exceptions
assert len(errors) == 1
assert errors[0].json_path == f"$.{field}"
assert errors[0].validator == "required"


@mark.parametrize("field", RESOURCE_SPROUT_REQUIRED_FIELDS.keys())
def test_raises_error_if_resource_required_field_is_missing(properties, field):
"""Should raise an error if a required field is missing among the resource
properties."""
del properties["resources"][0][field]

with raises(ExceptionGroup) as error_info:
check_properties(properties, check_required=True)

errors = error_info.value.exceptions
assert len(errors) == 1
assert errors[0].json_path == f"$.resources[0].{field}"
assert errors[0].validator == "required"


@mark.parametrize("check_required", [True, False])
@mark.parametrize("name,type", PACKAGE_SPROUT_REQUIRED_FIELDS.items())
def test_raises_error_if_package_field_is_blank(properties, name, type, check_required):
"""Should raise an error if there is one required package field that is present but
blank."""
properties[name] = get_blank_value_for_type(type)

with raises(ExceptionGroup) as error_info:
check_properties(properties, check_required=check_required)

blank_errors = [
error for error in error_info.value.exceptions if error.validator == "blank"
]

assert len(blank_errors) == 1
assert blank_errors[0].json_path == f"$.{name}"


@mark.parametrize("check_required", [True, False])
@mark.parametrize("name,type", RESOURCE_SPROUT_REQUIRED_FIELDS.items())
def test_raises_error_if_resource_field_is_blank(
properties, name, type, check_required
):
"""Should raise an error if there is one required resource field that is present
but blank."""
properties["resources"][0][name] = get_blank_value_for_type(type)

with raises(ExceptionGroup) as error_info:
check_properties(properties, check_required=check_required)

blank_errors = [
error for error in error_info.value.exceptions if error.validator == "blank"
]

assert len(blank_errors) == 1
assert blank_errors[0].json_path == f"$.resources[0].{name}"


@mark.parametrize("check_required", [True, False])
def test_raises_error_if_there_are_both_package_and_resource_errors(
properties, check_required
):
"""Should raise `CheckError`s if there are both package and resource errors."""
properties["name"] = "space in name"
properties["title"] = 123
properties["resources"][0]["name"] = "space in name"
properties["resources"][1]["path"] = "/bad path"
properties["resources"][1]["data"] = "some data"

with raises(ExceptionGroup) as error_info:
check_properties(properties, check_required=check_required)

errors = error_info.value.exceptions
assert [error.json_path for error in errors] == [
"$.name",
"$.resources[0].name",
"$.resources[1].data",
"$.resources[1].path",
"$.resources[1].path",
"$.title",
]
assert [error.validator for error in errors] == [
"pattern",
"pattern",
"inline-data",
"pattern",
"pattern",
"type",
]


@mark.parametrize("check_required", [True, False])
def test_raises_error_for_only_sprout_specific_errors(properties, check_required):
"""Errors should be triggered by only those Data Package standard violations that
are relevant for Sprout."""
properties["resources"][0]["path"] = 123

with raises(ExceptionGroup) as error_info:
check_properties(properties, check_required=check_required)

errors = error_info.value.exceptions
assert errors == (
CheckError("123 is not of type 'string'", "$.resources[0].path", "type"),
)
Loading