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

Block HED from appearing in sidecars #635

Merged
merged 4 commits into from
Mar 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 6 additions & 1 deletion hed/errors/error_messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,12 @@ def sidecar_error_unknown_column(column_name):


@hed_error(SidecarErrors.SIDECAR_HED_USED, actual_code=SidecarErrors.SIDECAR_INVALID)
def sidecar_hed_used():
def SIDECAR_HED_USED():
return "'HED' is a reserved name and cannot be used as a sidecar except in expected places."


@hed_error(SidecarErrors.SIDECAR_HED_USED_COLUMN, actual_code=SidecarErrors.SIDECAR_INVALID)
def SIDECAR_HED_USED_COLUMN():
return "'HED' is a reserved name and cannot be used as a sidecar column name"


Expand Down
3 changes: 2 additions & 1 deletion hed/errors/error_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,9 @@ class SidecarErrors:
INVALID_POUND_SIGNS_VALUE = 'invalidNumberPoundSigns'
INVALID_POUND_SIGNS_CATEGORY = 'tooManyPoundSigns'
UNKNOWN_COLUMN_TYPE = 'sidecarUnknownColumn'
SIDECAR_HED_USED = 'SIDECAR_HED_USED'
SIDECAR_HED_USED_COLUMN = 'SIDECAR_HED_USED_COLUMN'
SIDECAR_NA_USED = 'SIDECAR_NA_USED'
SIDECAR_HED_USED = 'SIDECAR_HED_USED'

class SchemaErrors:
HED_SCHEMA_DUPLICATE_NODE = 'HED_SCHEMA_DUPLICATE_NODE'
Expand Down
23 changes: 22 additions & 1 deletion hed/validator/sidecar_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,23 @@ def validate_structure(self, sidecar, error_handler):
error_handler.pop_error_context()
return all_validation_issues

@staticmethod
def _check_for_key(key, data):
if isinstance(data, dict):
if key in data:
return bool(data[key])
else:
for sub_data in data.values():
result = SidecarValidator._check_for_key(key, sub_data)
if result is not None:
return result
elif isinstance(data, list):
for sub_data in data:
result = SidecarValidator._check_for_key(key, sub_data)
if result is not None:
return result
return None

def _validate_column_structure(self, column_name, dict_for_entry, error_handler):
""" Checks primarily for type errors such as expecting a string and getting a list in a json sidecar.

Expand All @@ -93,13 +110,17 @@ def _validate_column_structure(self, column_name, dict_for_entry, error_handler)
"""
val_issues = []
if column_name in self.reserved_column_names:
val_issues += error_handler.format_error_with_context(SidecarErrors.SIDECAR_HED_USED)
val_issues += error_handler.format_error_with_context(SidecarErrors.SIDECAR_HED_USED_COLUMN)
return val_issues

column_type = Sidecar._detect_column_type(dict_for_entry=dict_for_entry)
if column_type is None:
val_issues += error_handler.format_error_with_context(SidecarErrors.UNKNOWN_COLUMN_TYPE,
column_name=column_name)
elif column_type == ColumnType.Ignore:
found_hed = self._check_for_key("HED", dict_for_entry)
if found_hed:
val_issues += error_handler.format_error_with_context(SidecarErrors.SIDECAR_HED_USED)
elif column_type == ColumnType.Categorical:
raw_hed_dict = dict_for_entry["HED"]
if not raw_hed_dict:
Expand Down
24 changes: 20 additions & 4 deletions spec_tests/test_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@
from hed.errors import ErrorHandler, get_printable_issue_string


known_errors = [
'SIDECAR_INVALID',
'CHARACTER_INVALID',
'COMMA_MISSING',
"DEF_EXPAND_INVALID",
"DEF_INVALID",
]

skip_tests = ["VERSION_DEPRECATED", "CHARACTER_INVALID", "STYLE_WARNING"]

Expand All @@ -30,6 +37,12 @@ def run_single_test(self, test_file):
test_info = json.load(fp)
for info in test_info:
error_code = info['error_code']
verify_code = False
if error_code in known_errors:
verify_code = True

# To be deprecated once we add this to all tests
self._verify_code = verify_code
if error_code in skip_tests:
print(f"Skipping {error_code} test")
continue
Expand Down Expand Up @@ -62,6 +75,13 @@ def report_result(self, expected_result, issues, error_code, description, name,
print(f"Passed '{test_type}' (which should fail) '{name}': {test}")
print(get_printable_issue_string(issues))
self.fail_count.append(name)
elif self._verify_code:
if any(issue['code'] == error_code for issue in issues):
return
print(f"{error_code}: {description}")
print(f"Failed '{test_type}' (unexpected errors found) '{name}': {test}")
print(get_printable_issue_string(issues))
self.fail_count.append(name)
else:
if issues:
print(f"{error_code}: {description}")
Expand All @@ -75,9 +95,6 @@ def _run_single_string_test(self, info, schema, def_dict, error_code, descriptio
for test in tests:
test_string = HedString(test, schema)

# This expand should not be required here.
def_dict.expand_def_tags(test_string)

issues = string_validator.run_basic_checks(test_string, False)
issues += string_validator.run_full_string_checks(test_string)
error_handler.add_context_and_filter(issues)
Expand All @@ -86,7 +103,6 @@ def _run_single_string_test(self, info, schema, def_dict, error_code, descriptio
def _run_single_sidecar_test(self, info, schema, def_dict, error_code, description, name, error_handler):
for result, tests in info.items():
for test in tests:
# Well this is a disaster
buffer = io.BytesIO(json.dumps(test).encode("utf-8"))
sidecar = Sidecar(buffer)
issues = sidecar.validate(hed_schema=schema, extra_def_dicts=def_dict, error_handler=error_handler)
Expand Down