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 simple helper functions #962

Merged
merged 33 commits into from
Jan 21, 2025
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
33 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
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
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
144b12f
apply suggestions from code review
martonvago Jan 15, 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
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
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
)


def check_resource_id_in_data_path(
def check_id_in_resource_path(
properties: dict, index: int | None = None
) -> list[CheckError]:
"""Checks if the data path in resource properties is well-formed.
Expand Down
4 changes: 2 additions & 2 deletions seedcase_sprout/core/sprout_checks/check_no_inline_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ def check_no_inline_data(
return [
CheckError(
message=(
"'data' should not be set. Sprout expects data in separate "
"data files specified by 'path'."
"Sprout doesn't use the 'data' field, instead it expects data "
"in separate files that are given in the 'path' field."
),
json_path=get_json_path_to_resource_field("data", index),
validator="inline-data",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
get_json_path_to_resource_field,
)

CHECKS_TYPE_ERROR_MESSAGE = "{field_value} is not of type '{field_type}'"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could arguably go into a separate file or even be exposed by the checks package. As #978 will rework this, I'll just leave it like this for now.


def check_data_path_string(

def check_resource_path_string(
properties: dict, index: int | None = None
) -> list[CheckError]:
"""Checks that the `path` field of a set of resource properties is of type string.
Expand All @@ -22,7 +24,9 @@ def check_data_path_string(

return [
CheckError(
message=f"{path} is not of type 'string'",
message=CHECKS_TYPE_ERROR_MESSAGE.format(
field_value=path, field_type="string"
),
json_path=get_json_path_to_resource_field("path", index),
validator="type",
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

from pytest import mark

from seedcase_sprout.core.sprout_checks.check_resource_id_in_data_path import (
check_resource_id_in_data_path,
from seedcase_sprout.core.sprout_checks.check_id_in_resource_path import (
check_id_in_resource_path,
)
from seedcase_sprout.core.sprout_checks.get_json_path_to_resource_field import (
get_json_path_to_resource_field,
Expand All @@ -15,13 +15,13 @@ def test_passes_if_data_path_well_formed(index):
"""Should pass if the path contains a resource ID."""
properties = {"path": str(Path("resources", "1", "data.parquet"))}

assert check_resource_id_in_data_path(properties, index) == []
assert check_id_in_resource_path(properties, index) == []


@mark.parametrize("index", [None, 2])
def test_passes_if_data_path_not_present(index):
"""Should pass if the path is not set."""
assert check_resource_id_in_data_path({}, index) == []
assert check_id_in_resource_path({}, index) == []


@mark.parametrize("index", [None, 2])
Expand All @@ -30,7 +30,7 @@ def test_passes_if_path_of_wrong_type(index, data_path):
"""Should pass if path is of the wrong type."""
properties = {"path": data_path}

assert check_resource_id_in_data_path(properties, index) == []
assert check_id_in_resource_path(properties, index) == []


@mark.parametrize("index", [None, 2])
Expand All @@ -47,9 +47,9 @@ def test_returns_error_if_data_path_is_malformed(index, data_path):
"""Returns list of `CheckError`s if the data path does not contain a resource ID."""
properties = {"path": str(data_path)}

errors = check_resource_id_in_data_path(properties, index)
errors = check_id_in_resource_path(properties, index)

assert len(errors) == 1
assert errors[0].message == "'path' should contain the resource ID"
assert errors[0].message
assert errors[0].json_path == get_json_path_to_resource_field("path", index)
assert errors[0].validator == "pattern"
2 changes: 1 addition & 1 deletion tests/core/sprout_checks/test_check_no_inline_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@ def test_error_found_if_data_is_set(index):
errors = check_no_inline_data(properties, index)

assert len(errors) == 1
assert "'data' should not be set" in errors[0].message
assert errors[0].message
assert errors[0].json_path == get_json_path_to_resource_field("data", index)
assert errors[0].validator == "inline-data"
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from pytest import mark

from seedcase_sprout.core.sprout_checks.check_data_path_string import (
check_data_path_string,
from seedcase_sprout.core.sprout_checks.check_resource_path_string import (
check_resource_path_string,
)
from seedcase_sprout.core.sprout_checks.get_json_path_to_resource_field import (
get_json_path_to_resource_field,
Expand All @@ -13,23 +13,23 @@ def test_passes_if_data_path_string(index):
"""Should pass if the path is of type string."""
properties = {"path": "a string"}

assert check_data_path_string(properties, index) == []
assert check_resource_path_string(properties, index) == []


@mark.parametrize("index", [None, 2])
def test_passes_if_data_path_not_present(index):
"""Should pass if the path is not set."""
assert check_data_path_string({}, index) == []
assert check_resource_path_string({}, index) == []


@mark.parametrize("index", [None, 2])
def test_error_found_if_path_not_string(index):
"""Should find an error if the path is not of type string."""
properties = {"path": 123}

errors = check_data_path_string(properties, index)
errors = check_resource_path_string(properties, index)

assert len(errors) == 1
assert errors[0].message == "123 is not of type 'string'"
assert "string" in errors[0].message
assert errors[0].json_path == get_json_path_to_resource_field("path", index)
assert errors[0].validator == "type"
Loading