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

Fix bug in column mapper dicts could be returned wrong #844

Merged
merged 1 commit into from
Jan 31, 2024
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
9 changes: 5 additions & 4 deletions hed/models/column_mapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from hed.models.sidecar import Sidecar
from hed.errors.error_reporter import ErrorHandler
from hed.errors.error_types import ValidationErrors
from hed.models.definition_dict import DefinitionDict

import copy
from collections import Counter
Expand Down Expand Up @@ -32,16 +33,16 @@ def __init__(self, sidecar=None, tag_columns=None, column_prefix_dictionary=None
the sidecar.
Notes:
- All column numbers are 0 based.
- All column numbers are 0 based.
- The column_prefix_dictionary may be deprecated/renamed in the future.
- These are no longer prefixes, but rather converted to value columns:
- These are no longer prefixes, but rather converted to value columns:
{"key": "Description", 1: "Label/"} will turn into value columns as
{"key": "Description/#", 1: "Label/#"}
It will be a validation issue if column 1 is called "key" in the above example.
This means it no longer accepts anything but the value portion only in the columns.
"""

# Maps column number to column_entry. This is what's actually used by most code.
self._final_column_map = {}
self._no_mapping_info = True
Expand Down Expand Up @@ -392,7 +393,7 @@ def get_def_dict(self, hed_schema, extra_def_dicts=None):
if self._sidecar:
return self._sidecar.get_def_dict(hed_schema=hed_schema, extra_def_dicts=extra_def_dicts)

return []
return DefinitionDict(extra_def_dicts, hed_schema=hed_schema)

def get_column_mapping_issues(self):
""" Get all the issues with finalizing column mapping(duplicate columns, missing required, etc)
Expand Down
31 changes: 17 additions & 14 deletions tests/models/test_column_mapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import os

from hed.models import ColumnMapper, ColumnType, HedString
from hed.models.sidecar import Sidecar
from hed.models.sidecar import Sidecar, DefinitionDict
from hed.errors import ValidationErrors
from hed import load_schema

Expand All @@ -24,26 +24,14 @@ def setUpClass(cls):

cls.base_data_dir = os.path.join(os.path.dirname(os.path.realpath(__file__)), '../data/')
cls.basic_events_json = os.path.join(cls.base_data_dir, "sidecar_tests/both_types_events.json")
cls.bids_events_defs = os.path.join(cls.base_data_dir, "validator_tests/bids_events.json")
cls.basic_event_name = "trial_type"
cls.basic_event_type = ColumnType.Categorical
cls.basic_hed_tags_column = "onset"
cls.basic_column_map = ["onset", "duration", "trial_type", "response_time", " stim_file"]
cls.basic_event_row = ["1.2", "0.6", "go", "1.435", "images/red_square.jpg"]
cls.basic_event_row_invalid = ["1.2", "0.6", "invalid_category_key", "1.435", "images/red_square.jpg"]

cls.add_column_name = "TestColumn"
cls.add_column_number = 0
cls.hed_string = "Event/Label/#"
cls.test_column_map = ["TestColumn"]
cls.required_prefix = "TestRequiredPrefix/"
cls.complex_hed_tag_required_prefix = \
"TestRequiredPrefix/ThisIsAHedTag, (TestRequiredPrefix/NewTag, TestRequiredPrefix/NewTag3)"
cls.complex_hed_tag_no_prefix = "ThisIsAHedTag,(NewTag,NewTag3)"

cls.short_tag_key = 'Item/Language-item/Character/'
cls.short_tag_with_missing_prefix = "D"
cls.short_tag_partial_prefix = 'Language-item/Character/'
cls.short_tag_partial_prefix2 = 'Character/'

def test_set_tag_columns(self):
mapper = ColumnMapper()
Expand Down Expand Up @@ -211,6 +199,21 @@ def test_tag_mapping_complex(self):
self.assertEqual(mapper._final_column_map[1].hed_dict, "Label/#")
self.assertEqual(mapper._final_column_map[2].column_type, ColumnType.HEDTags)

def test_get_def_dict(self):
mapper = ColumnMapper()
def_dict_empty = mapper.get_def_dict(self.hed_schema)
self.assertIsInstance(def_dict_empty, DefinitionDict)
def_dict_base = DefinitionDict("(Definition/TestDef, (Event))", self.hed_schema)
self.assertIsInstance(def_dict_base, DefinitionDict)
self.assertEqual(len(def_dict_base.defs), 1)
def_dict = mapper.get_def_dict(self.hed_schema, extra_def_dicts=def_dict_base)
self.assertIsInstance(def_dict, DefinitionDict)
self.assertEqual(len(def_dict.defs), 1)

mapper._set_sidecar(Sidecar(self.bids_events_defs))
def_dict_combined = mapper.get_def_dict(self.hed_schema, extra_def_dicts=def_dict_base)
self.assertIsInstance(def_dict_combined, DefinitionDict)
self.assertEqual(len(def_dict_combined.defs), 4)


if __name__ == '__main__':
Expand Down