Skip to content

Commit

Permalink
Block HED from appearing in sidecars (#635)
Browse files Browse the repository at this point in the history
* Block HED from appearing in sidecars
  • Loading branch information
IanCa authored Mar 23, 2023
1 parent 697791c commit 7cf7826
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 7 deletions.
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

0 comments on commit 7cf7826

Please sign in to comment.