diff --git a/docs/user_guide.md b/docs/user_guide.md index 292261d00..0dc419df5 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -223,7 +223,7 @@ issues = tabular.validate(schema) def_dict = tabular.get_def_dict(schema) ``` -Validation of a tabular file runs in four stages, and stops at the first stage that finds an error (a warning alone never stops it): +Validation of a tabular file runs in four stages, and stops at the first stage that finds an error (a warning alone never stops it). Before them, a header that repeats a column name (`trial`, `trial`) gives one `TABULAR_INVALID` per repeated name and nothing else; `tabular.duplicate_columns` lists those names. Column names are the header cells with surrounding whitespace removed. 1. **Sidecar**: every template and categorical annotation in the sidecar. Pass `validate_sidecar=False` if you have already validated the sidecar yourself. 2. **Column values**: each value column's distinct values against the units or value class of the template's `#` tag, and each categorical column's values against its sidecar keys. A value that appears in many rows is checked once and reported at its first row, with the number of rows in the issue's `row_count`. diff --git a/hed/errors/error_messages.py b/hed/errors/error_messages.py index 2110389ad..1368e3703 100644 --- a/hed/errors/error_messages.py +++ b/hed/errors/error_messages.py @@ -237,6 +237,11 @@ def val_error_extra_column(column_name): ) +@hed_error(ValidationErrors.TABULAR_INVALID) +def val_error_tabular_invalid(column_name): + return f"Column '{column_name}' appears more than once in the first line (the column names) of the tabular file." + + @hed_error(ValidationErrors.SIDECAR_AND_OTHER_COLUMNS) def val_error_sidecar_with_column(column_names): return f"You cannot use a column name in curly braces and to designate a tag column. Found {column_names}." diff --git a/hed/errors/error_types.py b/hed/errors/error_types.py index c605a8054..40fac97f3 100644 --- a/hed/errors/error_types.py +++ b/hed/errors/error_types.py @@ -44,6 +44,7 @@ class ValidationErrors: SIDECAR_KEY_MISSING = "SIDECAR_KEY_MISSING" HED_COLUMN_MISSING = "HED_COLUMN_MISSING" STYLE_WARNING = "STYLE_WARNING" + TABULAR_INVALID = "TABULAR_INVALID" TAG_EMPTY = "TAG_EMPTY" TAG_EXPRESSION_REPEATED = "TAG_EXPRESSION_REPEATED" TAG_EXTENDED = "TAG_EXTENDED" diff --git a/hed/models/base_input.py b/hed/models/base_input.py index 9187d47a6..b18466ab7 100644 --- a/hed/models/base_input.py +++ b/hed/models/base_input.py @@ -2,6 +2,7 @@ Superclass representing a basic columnar file. """ +import io import os import openpyxl @@ -53,11 +54,14 @@ def __init__( - file is blank. - An invalid dataframe was passed with size 0. - An invalid extension was provided. - - A duplicate or empty column name appears. + - A blank column name appears and allow_blank_names is False (BAD_COLUMN_NAMES). - Cannot open the indicated file. - The specified worksheet name does not exist. - If the sidecar file or tabular file had invalid format and could not be read. + A repeated column name does not raise: it is recorded in ``duplicate_columns`` and ``validate`` + reports it as TABULAR_INVALID. + """ if mapper is None: mapper = ColumnMapper() @@ -68,6 +72,7 @@ def __init__( self._loaded_workbook = None self._worksheet_name = worksheet_name self._dataframe = None + self._duplicate_columns = [] input_type = file_type if isinstance(file, str): @@ -82,7 +87,7 @@ def __init__( if column_issues: raise HedFileError( HedExceptions.BAD_COLUMN_NAMES, - "Duplicate or blank columns found. See issues.", + "Blank column names found. See issues.", self.name, issues=column_issues, ) @@ -165,6 +170,16 @@ def has_column_names(self) -> bool: """True if dataframe has column names.""" return self._has_column_names + @property + def duplicate_columns(self) -> list[str]: + """The column names that appear more than once in the header, each once, in order of its second appearance. + + Names are compared after surrounding whitespace is removed; blank names are not counted. Read from the + header itself, so a repeated name is listed under its own name, not the suffixed name the loaded dataframe + gives its second column. Empty when the file has no column names. + """ + return self._duplicate_columns + @property def loaded_workbook(self): """The underlying loaded workbooks.""" @@ -279,7 +294,9 @@ def to_csv(self, file=None) -> str | None: def columns(self) -> list[str]: """Returns a list of the column names. - Empty if no column names. + Empty if no column names. The names are the header cells with surrounding whitespace removed; + a name repeated in the header is suffixed in the second column it names (``trial.1``), as pandas does, + and is listed in ``duplicate_columns``. Returns: list: The column names. @@ -371,8 +388,9 @@ def validate( ) -> list[dict]: """Validate this file with a SpreadsheetValidator and return all issues. - The validator runs four stages and stops at the first stage that finds an error: the sidecar, - each column's own values, the HED column, and the assembled rows. See + The validator runs five stages and stops at the first stage that finds an error: the column names + (a repeated name is TABULAR_INVALID), the sidecar, each column's own values, the HED column, and the + assembled rows. See :class:`~hed.validator.spreadsheet_validator.SpreadsheetValidator`. Parameters: @@ -449,6 +467,95 @@ def _dataframe_has_names(dataframe) -> bool: return True return False + @staticmethod + def _trimmed_names(names) -> list: + """Return the names with surrounding whitespace removed; a label that is not a string is unchanged.""" + return [name.strip() if isinstance(name, str) else name for name in names] + + @staticmethod + def _repeated_names(names) -> list[str]: + """Return each non-blank string name that appears more than once, in order of its second appearance.""" + seen = set() + repeated = [] + for name in names: + if not isinstance(name, str) or not name: + continue + if name in seen and name not in repeated: + repeated.append(name) + seen.add(name) + return repeated + + @staticmethod + def _unique_names(names) -> list: + """Return the names with each repeat of a string name suffixed ``.1``, ``.2``, ... as pandas reads a header.""" + taken = set() + unique = [] + for name in names: + new_name = name + suffix = 0 + while isinstance(name, str) and new_name in taken: + suffix += 1 + new_name = f"{name}.{suffix}" + taken.add(new_name) + unique.append(new_name) + return unique + + def _set_column_names(self, names, header_cells=None): + """Name the dataframe's columns with the trimmed names and record the repeated ones. + + Parameters: + names (list): The dataframe's column labels as loaded. + header_cells (list or None): The cells of a text file's header record, from which the repeats are + read because pandas has already suffixed them in names. None reads the repeats from names. + """ + names = self._trimmed_names(names) + self._duplicate_columns = self._repeated_names( + names if header_cells is None else self._trimmed_names(header_cells) + ) + self._dataframe.columns = self._unique_names(names) + + @staticmethod + def _read_header_cells(file): + """Return the cells of a text file's header record and the file to hand to pandas. + + The header is read by pandas as a data row (``header=None``), so its cells are parsed exactly as the + dataframe's header is (blank lines skipped, BOM removed, quotes removed, a quoted cell may span + lines) but are not yet suffixed. A file-like object is returned to where it was; one that cannot + seek or report its position is read into memory first. + + Parameters: + file (str, os.PathLike or file-like): Path to the text file or a binary or text file-like object. + + Returns: + tuple: The header cells (list[str]) and the file or in-memory copy for pandas. + """ + if isinstance(file, (str, os.PathLike)): + return BaseInput._first_record_cells(file), file + position = None + if hasattr(file, "seekable") and file.seekable(): + try: + position = file.tell() + except OSError: + pass # A text file advanced with next() cannot tell its position, but can still be read. + if position is None: + contents = file.read() + file = io.BytesIO(contents) if isinstance(contents, bytes) else io.StringIO(contents) + position = 0 + cells = BaseInput._first_record_cells(file) + file.seek(position) + return cells, file + + @staticmethod + def _first_record_cells(file) -> list[str]: + """Return the cells of the first record pandas reads from file, or [] if there is none.""" + try: + first = pd.read_csv( + file, delimiter="\t", header=None, nrows=1, dtype=str, keep_default_na=False, skip_blank_lines=True + ) + except pd.errors.EmptyDataError: + return [] + return list(first.iloc[0]) if len(first) else [] + def assemble(self, mapper=None, skip_curly_braces=False) -> pd.DataFrame: """Assembles the HED strings. @@ -580,6 +687,8 @@ def _open_dataframe_file(self, file, has_column_names, input_type): # keeps it missing. self._dataframe = file.astype(object).fillna("n/a").astype(str) self._has_column_names = self._dataframe_has_names(self._dataframe) + if self._has_column_names: + self._set_column_names(self._dataframe.columns) return # Check for empty file or None @@ -627,6 +736,8 @@ def _load_excel_file(self, file, has_column_names): self._loaded_workbook = openpyxl.load_workbook(file) loaded_worksheet = self.get_worksheet(self._worksheet_name) self._dataframe = self._get_dataframe_from_worksheet(loaded_worksheet, has_column_names) + if has_column_names: + self._set_column_names(self._dataframe.columns) except Exception as e: raise HedFileError( HedExceptions.INVALID_FILE_FORMAT, f"Failed to load Excel file: {str(e)}", self.name @@ -659,12 +770,17 @@ def _load_text_file(self, file, pandas_header): - Skips blank lines during parsing - Uses specific na_values configuration ("", "null") - Handles pandas.errors.EmptyDataError for files with no data + - With a header, the header record is read first so that repeated names are known before + pandas suffixes them (``duplicate_columns``); the column names are the trimmed header cells """ if isinstance(file, str) and os.path.exists(file) and os.path.getsize(file) == 0: self._dataframe = pd.DataFrame() # Handle empty file return try: + header_cells = None + if pandas_header is not None: + header_cells, file = self._read_header_cells(file) self._dataframe = pd.read_csv( file, delimiter="\t", @@ -678,6 +794,8 @@ def _load_text_file(self, file, pandas_header): # None and NULL, which pandas would otherwise read as missing, is a value and is validated as one, # as it is when a DataFrame is passed in. self._dataframe = self._dataframe.fillna("n/a") + if header_cells is not None: + self._set_column_names(self._dataframe.columns, header_cells) except pd.errors.EmptyDataError: self._dataframe = pd.DataFrame() # Handle case where file has no data except Exception as e: diff --git a/hed/models/column_mapper.py b/hed/models/column_mapper.py index e31ff22d3..67f596d21 100644 --- a/hed/models/column_mapper.py +++ b/hed/models/column_mapper.py @@ -80,13 +80,13 @@ def __init__( self._warn_on_missing_column = warn_on_missing_column if tag_columns is None: tag_columns = [] - self._tag_columns = tag_columns + self._tag_columns = self._trimmed_selectors(tag_columns) if optional_tag_columns is None: optional_tag_columns = [] - self._optional_tag_columns = optional_tag_columns + self._optional_tag_columns = self._trimmed_selectors(optional_tag_columns) if column_prefix_dictionary is None: column_prefix_dictionary = {} - self._column_prefix_dictionary = column_prefix_dictionary + self._column_prefix_dictionary = self._trimmed_selectors(column_prefix_dictionary) self._na_patterns = ["n/a", "nan"] self._sidecar = None @@ -154,7 +154,8 @@ def check_for_blank_names(column_map, allow_blank_names) -> list[dict]: Returns: list[dict]: A list of dicts, one per issue. """ - # We don't have any checks right now if blank/duplicate is allowed + # Blank names are allowed unless the caller says otherwise; repeated names are not checked here + # (BaseInput.duplicate_columns records them and validation reports TABULAR_INVALID). if allow_blank_names: return [] @@ -226,8 +227,8 @@ def set_tag_columns(self, tag_columns=None, optional_tag_columns=None, finalize_ tag_columns = [] if optional_tag_columns is None: optional_tag_columns = [] - self._tag_columns = tag_columns - self._optional_tag_columns = optional_tag_columns + self._tag_columns = self._trimmed_selectors(tag_columns) + self._optional_tag_columns = self._trimmed_selectors(optional_tag_columns) if finalize_mapping: self._finalize_mapping() @@ -252,10 +253,24 @@ def set_column_map(self, new_column_map=None): def set_column_prefix_dictionary(self, column_prefix_dictionary, finalize_mapping=True): """Set the column prefix dictionary.""" - self._column_prefix_dictionary = column_prefix_dictionary + self._column_prefix_dictionary = self._trimmed_selectors(column_prefix_dictionary) if finalize_mapping: self._finalize_mapping() + @staticmethod + def _trimmed_selectors(selectors): + """Return column selectors with each name trimmed, as BaseInput trims header names; numbers are unchanged. + + Parameters: + selectors (list or dict): Column numbers or names, or a dict keyed by them. + + Returns: + list or dict: A new list, or a new dict with the same values. + """ + if isinstance(selectors, dict): + return {key.strip() if isinstance(key, str) else key: value for key, value in selectors.items()} + return [item.strip() if isinstance(item, str) else item for item in selectors] + @staticmethod def _get_sidecar_basic_map(column_map, column_data): basic_final_map = {} diff --git a/hed/tools/bids/bids_file_group.py b/hed/tools/bids/bids_file_group.py index f76f73fd6..58e43b9b0 100644 --- a/hed/tools/bids/bids_file_group.py +++ b/hed/tools/bids/bids_file_group.py @@ -8,6 +8,7 @@ import pandas as pd from hed.errors.error_reporter import ErrorHandler, check_for_any_errors +from hed.errors.exceptions import HedFileError from hed.tools.analysis.tabular_summary import TabularSummary from hed.tools.bids.bids_sidecar_file import BidsSidecarFile from hed.tools.bids.bids_tabular_file import BidsTabularFile @@ -195,6 +196,7 @@ def validate_datafiles(self, hed_schema, extra_def_dicts=None, error_handler=Non logger.debug(f"Validating data file {i}/{len(hed_files)}: {os.path.basename(data_obj.file_path)}") sidecar = data_obj.sidecar + sidecar_has_errors = False if sidecar is not None and id(sidecar) not in group_sidecars: key = json.dumps(sidecar.loaded_dict, sort_keys=True) if key not in merged_sidecar_errors: @@ -203,12 +205,24 @@ def validate_datafiles(self, hed_schema, extra_def_dicts=None, error_handler=Non ) merged_sidecar_errors[key] = check_for_any_errors(sidecar_issues) issues += sidecar_issues - if merged_sidecar_errors[key]: - logger.debug(f"Skipping {os.path.basename(data_obj.file_path)}: its merged sidecar has errors") - continue + sidecar_has_errors = merged_sidecar_errors[key] had_contents = data_obj.contents - data_obj.set_contents(overwrite=False) + # A file whose merged sidecar has errors is skipped, unless its header repeats a column name: + # validate then reports only TABULAR_INVALID (stage 0) and checks no row against the sidecar. + # A file that cannot even be loaded (a blank column name) is skipped as it was before loading. + try: + data_obj.set_contents(overwrite=False) + except HedFileError: + if not sidecar_has_errors: + raise + logger.debug(f"Skipping {os.path.basename(data_obj.file_path)}: its merged sidecar has errors") + continue + if sidecar_has_errors and not data_obj.contents.duplicate_columns: + logger.debug(f"Skipping {os.path.basename(data_obj.file_path)}: its merged sidecar has errors") + if not had_contents: + data_obj.clear_contents() + continue file_issues = data_obj.contents.validate( hed_schema, extra_def_dicts=extra_def_dicts, @@ -267,7 +281,8 @@ def _make_datafile_dict(self, root_path, tsv_list): continue tsv_obj.set_sidecar(self._get_tsv_sidecar(root_path, tsv_obj)) try: - column_headers = list(pd.read_csv(file_path, sep="\t", nrows=0).columns) + # Trimmed as BaseInput trims its column names, so a header cell "HED " is the HED column. + column_headers = [str(name).strip() for name in pd.read_csv(file_path, sep="\t", nrows=0).columns] except Exception as e: self.bad_files[file_path] = f"{file_path} does not have a valid column header: {str(e)}" continue diff --git a/hed/validator/spreadsheet_validator.py b/hed/validator/spreadsheet_validator.py index e8bfa69e4..f257cba61 100644 --- a/hed/validator/spreadsheet_validator.py +++ b/hed/validator/spreadsheet_validator.py @@ -1,4 +1,4 @@ -"""Validates HED annotations in tabular data in four stopping stages.""" +"""Validates HED annotations in tabular data in stopping stages.""" from __future__ import annotations @@ -33,10 +33,13 @@ class SpreadsheetValidator: - """Validates HED annotations in tabular data against a HED schema, in four stages. + """Validates HED annotations in tabular data against a HED schema, in four stages after a header check. The stages run in order and each stops the run when it finds an error (never on a warning alone): + 0. **Column names**: a :class:`~hed.models.base_input.BaseInput` whose header repeats a column name gets + one TABULAR_INVALID per repeated name (``duplicate_columns``) and nothing else: neither the sidecar + nor any row is checked or assembled (specification 3.2.10). Other column sources cannot repeat a name. 1. **Sidecar**: every template and categorical string of the sidecar (``SidecarValidator``). A sidecar string is a fragment of a row, so the required-tag check waits for assembly. 2. **Column values**: the column structure (mapper issues, sidecar references to columns the table @@ -97,7 +100,10 @@ def validate( validate_sidecar=True, def_dicts=None, ) -> list[dict]: - """Run the four stages on a column source, stopping at the first stage that finds an error. + """Run the stages on a column source, stopping at the first stage that finds an error. + + A BaseInput whose header repeats a column name is reported by stage 0 alone: one TABULAR_INVALID per + repeated name, in order of its second appearance, with the COLUMN and (when ``name``) FILE_NAME context. Parameters: source (ColumnSource): The table to validate. A BaseInput (TabularInput, SpreadsheetInput) or any @@ -159,6 +165,10 @@ def validate( # Strings and lists of strings need the schema to become definitions. extra_def_dicts = DefinitionDict(extra_def_dicts, hed_schema=self._schema) + # Stage 0: a repeated column name stops the run before the sidecar, as in hed-javascript. + if isinstance(source, BaseInput) and source.duplicate_columns: + return self._report_duplicate_columns(source.duplicate_columns, name, error_handler) + issues = [] # Stage 1 runs before this table's FILE_NAME context is pushed so that its issues carry the # sidecar's own name. With no name here, the caller manages the location context for both. @@ -182,6 +192,22 @@ def validate( if name: error_handler.pop_error_context() + @staticmethod + def _report_duplicate_columns(column_names, name, error_handler) -> list[dict]: + """Stage 0: one TABULAR_INVALID per repeated column name, kept in the given order (not sorted).""" + issues = [] + if name: + error_handler.push_error_context(ErrorContext.FILE_NAME, name) + try: + for column_name in column_names: + error_handler.push_error_context(ErrorContext.COLUMN, column_name) + issues += error_handler.format_error_with_context(ValidationErrors.TABULAR_INVALID, column_name) + error_handler.pop_error_context() + finally: + if name: + error_handler.pop_error_context() + return issues + def _validate_source(self, source, sidecar, extra_def_dicts, error_handler, row_offset, issues) -> list[dict]: """Stages 2 to 4, run with the FILE_NAME context (if any) already pushed.""" if sidecar is not None: diff --git a/spec_tests/hed-tests b/spec_tests/hed-tests index e2753fb51..80a17f0f8 160000 --- a/spec_tests/hed-tests +++ b/spec_tests/hed-tests @@ -1 +1 @@ -Subproject commit e2753fb512da50cb624d6df5795f2cc8c36ea4df +Subproject commit 80a17f0f8010d544e3536d1d1eef57b3a9ee0afd diff --git a/tests/data/spreadsheet_validator_tests/duplicate_columns.tsv b/tests/data/spreadsheet_validator_tests/duplicate_columns.tsv new file mode 100644 index 000000000..dea850028 --- /dev/null +++ b/tests/data/spreadsheet_validator_tests/duplicate_columns.tsv @@ -0,0 +1,3 @@ +onset duration trial trial +1.0 0 go stop +2.0 0 stop go diff --git a/tests/models/test_base_input.py b/tests/models/test_base_input.py index 3f9a523c6..e73f5a17b 100644 --- a/tests/models/test_base_input.py +++ b/tests/models/test_base_input.py @@ -1,6 +1,7 @@ import io import os import shutil +import socket import unittest import numpy as np @@ -194,3 +195,135 @@ def test_combine_dataframe_with_mixed_values(self): result = BaseInput.combine_dataframe(loaded_df) expected = pd.Series(["apple, guitar", "elephant, harmonica", "cherry, fox", "", ""]) self.assertTrue(result.equals(expected)) + + +class TestDuplicateColumns(unittest.TestCase): + """duplicate_columns: the names a header repeats, read before pandas suffixes them (TABULAR_INVALID).""" + + @staticmethod + def _text(header, rows=("1\t2\t3\t4",)): + return "\n".join([header, *rows]) + "\n" + + def test_from_path(self): + path = os.path.realpath( + os.path.join(os.path.dirname(__file__), "../data/spreadsheet_validator_tests/duplicate_columns.tsv") + ) + input_file = BaseInput(path) + self.assertEqual(input_file.duplicate_columns, ["trial"]) + # The dataframe is still loaded; pandas' suffix names the second column internally. + self.assertEqual(input_file.columns, ["onset", "duration", "trial", "trial.1"]) + self.assertEqual(list(input_file.dataframe["trial.1"]), ["stop", "go"]) + + def test_from_bytes_and_string_io(self): + text = self._text("onset\tduration\ttrial\ttrial") + for file in (io.BytesIO(text.encode("utf-8")), io.StringIO(text)): + input_file = BaseInput(file, file_type=".tsv") + self.assertEqual(input_file.duplicate_columns, ["trial"]) + self.assertEqual(input_file.columns, ["onset", "duration", "trial", "trial.1"]) + self.assertEqual(len(input_file.dataframe), 1) + + def test_file_like_read_from_its_position(self): + text = self._text("onset\tduration\ttrial\ttrial") + file = io.StringIO("skipped\n" + text) + file.readline() + input_file = BaseInput(file, file_type=".tsv") + self.assertEqual(input_file.duplicate_columns, ["trial"]) + self.assertEqual(len(input_file.dataframe), 1) + + def test_from_non_seekable_stream(self): + # A socket file cannot seek on any platform (a pipe can, inside its buffer, on Windows). + sender, receiver = socket.socketpair() + with sender, receiver: + sender.sendall(self._text("trial\tHED\ttrial\tHED").encode("utf-8")) + sender.shutdown(socket.SHUT_WR) + with receiver.makefile("rb") as reader: + self.assertFalse(reader.seekable()) + input_file = BaseInput(reader, file_type=".tsv") + self.assertEqual(input_file.duplicate_columns, ["trial", "HED"]) + self.assertEqual(input_file.columns, ["trial", "HED", "trial.1", "HED.1"]) + self.assertEqual(len(input_file.dataframe), 1) + + def test_text_stream_advanced_with_next(self): + # TextIOWrapper.tell() fails after next(); the rest of the stream is still read from where it is. + text = "preamble\n" + self._text("trial\ttrial\tHED", ("a\tb\tRed",)) + stream = io.TextIOWrapper(io.BytesIO(text.encode("utf-8")), encoding="utf-8") + next(stream) + input_file = BaseInput(stream, file_type=".tsv") + self.assertEqual(input_file.duplicate_columns, ["trial"]) + self.assertEqual(input_file.columns, ["trial", "trial.1", "HED"]) + self.assertEqual(len(input_file.dataframe), 1) + + def test_bom_blank_line_and_crlf(self): + text = "\ufeff\r\nonset\tduration\ttrial\ttrial\r\n1\t2\t3\t4\r\n" + input_file = BaseInput(io.BytesIO(text.encode("utf-8")), file_type=".tsv") + self.assertEqual(input_file.duplicate_columns, ["trial"]) + self.assertEqual(input_file.columns, ["onset", "duration", "trial", "trial.1"]) + + def test_names_are_trimmed(self): + input_file = BaseInput(io.StringIO(self._text("onset \t duration\ttrial \ttrial")), file_type=".tsv") + self.assertEqual(input_file.duplicate_columns, ["trial"]) + self.assertEqual(input_file.columns, ["onset", "duration", "trial", "trial.1"]) + unique = BaseInput(io.StringIO(self._text("onset \tduration\ttrial \tHED")), file_type=".tsv") + self.assertEqual(unique.duplicate_columns, []) + self.assertEqual(unique.columns, ["onset", "duration", "trial", "HED"]) + + def test_quoted_header_cells(self): + # The header is parsed as pandas parses it: quotes are removed before names are compared. + mixed = BaseInput(io.StringIO(self._text('trial\t"trial"\tHED', ("a\tb\tRed",))), file_type=".tsv") + self.assertEqual(mixed.duplicate_columns, ["trial"]) + self.assertEqual(mixed.columns, ["trial", "trial.1", "HED"]) + quoted = BaseInput(io.StringIO(self._text('"trial"\t"trial"\tHED', ("a\tb\tRed",))), file_type=".tsv") + self.assertEqual(quoted.duplicate_columns, ["trial"]) + self.assertEqual(quoted.columns, ["trial", "trial.1", "HED"]) + + def test_multiline_quoted_header(self): + # A quoted cell may hold a line break; the header record then spans two lines. + text = self._text('"trial\nnote"\ttrial\ttrial', ("a\tb\tc",)) + input_file = BaseInput(io.BytesIO(text.encode("utf-8")), file_type=".tsv") + self.assertEqual(input_file.duplicate_columns, ["trial"]) + self.assertEqual(input_file.columns, ["trial\nnote", "trial", "trial.1"]) + self.assertEqual(len(input_file.dataframe), 1) + + def test_order_and_count(self): + input_file = BaseInput(io.StringIO(self._text("b\ta\ta\tb\tb")), file_type=".tsv") + # Each name once, in order of its second appearance. + self.assertEqual(input_file.duplicate_columns, ["a", "b"]) + self.assertEqual(input_file.columns, ["b", "a", "a.1", "b.1", "b.2"]) + + def test_no_repeats(self): + input_file = BaseInput(io.StringIO(self._text("trial\ttrial_type\tHED", ("1\t2\t3",))), file_type=".tsv") + self.assertEqual(input_file.duplicate_columns, []) + self.assertEqual(self._blank_repeats().duplicate_columns, []) + + def _blank_repeats(self): + # Blank names have their own rule (allow_blank_names) and are not counted as repeats. + return BaseInput(io.StringIO(self._text("onset\t\t\tHED")), file_type=".tsv") + + def test_no_column_names(self): + input_file = BaseInput( + io.StringIO(self._text("trial\ttrial\ttrial\ttrial")), file_type=".tsv", has_column_names=False + ) + self.assertEqual(input_file.duplicate_columns, []) + self.assertEqual(len(input_file.dataframe), 2) + + def test_from_dataframe(self): + frame = pd.DataFrame([["1", "go", "stop"]], columns=["onset", "trial ", "trial"]) + input_file = BaseInput(frame) + self.assertEqual(input_file.duplicate_columns, ["trial"]) + self.assertEqual(input_file.columns, ["onset", "trial", "trial.1"]) + # The caller's frame keeps its own labels. + self.assertEqual(list(frame.columns), ["onset", "trial ", "trial"]) + + def test_from_excel(self): + import openpyxl + + workbook = openpyxl.Workbook() + worksheet = workbook.active + worksheet.append(["onset", "trial", "HED ", "trial", "HED"]) + worksheet.append(["1", "go", "Red", "stop", "Blue"]) + buffer = io.BytesIO() + workbook.save(buffer) + buffer.seek(0) + input_file = BaseInput(buffer, file_type=".xlsx") + self.assertEqual(input_file.duplicate_columns, ["trial", "HED"]) + self.assertEqual(input_file.columns, ["onset", "trial", "HED", "trial.1", "HED.1"]) diff --git a/tests/models/test_spreadsheet_input.py b/tests/models/test_spreadsheet_input.py index 595ada926..27eed9b11 100644 --- a/tests/models/test_spreadsheet_input.py +++ b/tests/models/test_spreadsheet_input.py @@ -8,6 +8,8 @@ from hed import schema from hed.errors import HedFileError from hed.models import Sidecar, SpreadsheetInput, TabularInput +from hed.models.base_input import BaseInput +from hed.models.column_mapper import ColumnMapper class Test(unittest.TestCase): @@ -245,5 +247,41 @@ def test_to_excel_workbook_no_col_names(self): self.assertTrue(excel_book.dataframe.equals(reloaded_df.dataframe)) +class TestTrimmedSelectors(unittest.TestCase): + """Header names are trimmed, so name selectors are trimmed too and still select their columns.""" + + @classmethod + def setUpClass(cls): + cls.schema = schema.load_schema_version("8.4.0") + cls.text = " HED tags \t Event label \tother\nRed\tgo\tx\n" + + def _input(self, **kwargs): + return SpreadsheetInput(io.BytesIO(self.text.encode("utf-8")), file_type=".tsv", **kwargs) + + def test_tag_columns(self): + spreadsheet = self._input(tag_columns=[" HED tags "]) + self.assertEqual(spreadsheet.columns, ["HED tags", "Event label", "other"]) + self.assertEqual(spreadsheet._mapper.get_tag_columns(), ["HED tags"]) + self.assertEqual(spreadsheet.validate(self.schema), []) + self.assertEqual(list(spreadsheet.series_a), ["Red"]) + + def test_optional_tag_columns_and_setters(self): + mappers = [ColumnMapper(optional_tag_columns=[" HED tags "]), ColumnMapper()] + mappers[1].set_tag_columns(optional_tag_columns=["HED tags "], finalize_mapping=False) + for mapper in mappers: + base_input = BaseInput(io.BytesIO(self.text.encode("utf-8")), file_type=".tsv", mapper=mapper) + self.assertEqual(base_input._mapper.get_tag_columns(), ["HED tags"]) + self.assertEqual(list(base_input.series_a), ["Red"]) + + def test_column_prefix_dictionary(self): + spreadsheet = self._input(tag_columns=["HED tags "], column_prefix_dictionary={" Event label": "Label/"}) + self.assertEqual(spreadsheet.validate(self.schema), []) + self.assertEqual(sorted(spreadsheet.series_a[0].split(", ")), ["Label/go", "Red"]) + mapper = ColumnMapper(tag_columns=["HED tags"]) + mapper.set_column_prefix_dictionary({"Event label ": "Label/"}, finalize_mapping=False) + base_input = BaseInput(io.BytesIO(self.text.encode("utf-8")), file_type=".tsv", mapper=mapper) + self.assertEqual(sorted(base_input.series_a[0].split(", ")), ["Label/go", "Red"]) + + if __name__ == "__main__": unittest.main() diff --git a/tests/tools/bids/test_bids_dataset.py b/tests/tools/bids/test_bids_dataset.py index c7858c44b..cb393fd1c 100644 --- a/tests/tools/bids/test_bids_dataset.py +++ b/tests/tools/bids/test_bids_dataset.py @@ -1,5 +1,6 @@ import os import shutil +import tempfile import unittest from hed.models.sidecar import Sidecar @@ -331,6 +332,22 @@ def test_get_summary_libraries(self): ) self.assertTrue("dataset" in summary2) + def test_validate_repeated_column(self): + """A repeated column name in one events file is TABULAR_INVALID for that file and nothing else.""" + with tempfile.TemporaryDirectory() as temp_dir: + root = os.path.join(temp_dir, "dataset") + shutil.copytree(self.root_path, root) + events = os.path.join(root, "sub-002", "eeg", "sub-002_task-FacePerception_run-1_events.tsv") + with open(events, encoding="utf-8", newline="") as fp: + header, rest = fp.read().split("\n", 1) + self.assertIn("\trep_lag\t", header) + with open(events, "w", encoding="utf-8", newline="") as fp: + fp.write(header.replace("\trep_lag\t", "\ttrial\t") + "\n" + rest) + issues = BidsDataset(root).validate(check_for_warnings=False) + self.assertEqual([issue["code"] for issue in issues], ["TABULAR_INVALID"]) + self.assertEqual(issues[0]["ec_column"], "trial") + self.assertTrue(issues[0]["ec_filename"].endswith("sub-002_task-FacePerception_run-1_events.tsv")) + if __name__ == "__main__": unittest.main() diff --git a/tests/tools/bids/test_bids_file_group.py b/tests/tools/bids/test_bids_file_group.py index b46d1e3fd..910962695 100644 --- a/tests/tools/bids/test_bids_file_group.py +++ b/tests/tools/bids/test_bids_file_group.py @@ -1,4 +1,6 @@ import os +import shutil +import tempfile import unittest from hed.errors.error_reporter import ErrorHandler @@ -92,6 +94,89 @@ def test_merged_sidecars_validated_once(self): self.assertRegex(issues[0]["ec_filename"], r"sub-002_task-FacePerception_run-[123]_events") self.assertEqual(issues[0]["ec_sidecarColumnName"], "bad_column") + def test_repeated_column_reported_despite_bad_merged_sidecar(self): + """A file whose merged sidecar has errors is skipped, but a repeated column name in its header is + still TABULAR_INVALID, and its rows are not checked against the bad sidecar.""" + source = os.path.realpath( + os.path.join(os.path.dirname(__file__), "../../data/bids_tests/eeg_ds003645s_hed_inheritance") + ) + with tempfile.TemporaryDirectory() as temp_dir: + root_path = os.path.join(temp_dir, "dataset") + shutil.copytree(source, root_path) + events_path = os.path.join(root_path, "sub-002", "sub-002_task-FacePerception_run-1_events.tsv") + with open(events_path, encoding="utf-8", newline="") as fp: + header, rest = fp.read().split("\n", 1) + self.assertIn("\trep_lag\t", header) + with open(events_path, "w", encoding="utf-8", newline="") as fp: + fp.write(header.replace("\trep_lag\t", "\ttrial\t") + "\n" + rest) + + file_paths = io_util.get_file_list( + root_path, extensions=[".tsv", ".json"], exclude_dirs=self.exclude_dirs, name_suffix=["_events"] + ) + events = BidsFileGroup(root_path, file_paths, "events") + group_sidecars = {id(sidecar.contents) for sidecar in events.sidecar_dict.values()} + for data_obj in events.datafile_dict.values(): + if "sub-002" in data_obj.file_path and id(data_obj.sidecar) not in group_sidecars: + data_obj.sidecar.loaded_dict["bad_column"] = {"HED": {"x": "InvalidTagXYZ"}} + issues = events.validate_datafiles(load_schema_version("8.4.0")) + + self.assertEqual(sorted(issue["code"] for issue in issues), ["TABULAR_INVALID", "TAG_INVALID"]) + repeated = next(issue for issue in issues if issue["code"] == "TABULAR_INVALID") + self.assertEqual(repeated["ec_column"], "trial") + self.assertTrue(repeated["ec_filename"].endswith("sub-002_task-FacePerception_run-1_events.tsv")) + + def test_unloadable_file_with_bad_merged_sidecar_is_skipped(self): + """A file whose merged sidecar has errors and whose header has a blank name (TabularInput refuses + to load it) is skipped as before: validation does not raise and the sidecar issue is returned.""" + source = os.path.realpath( + os.path.join(os.path.dirname(__file__), "../../data/bids_tests/eeg_ds003645s_hed_inheritance") + ) + with tempfile.TemporaryDirectory() as temp_dir: + root_path = os.path.join(temp_dir, "dataset") + shutil.copytree(source, root_path) + events_path = os.path.join(root_path, "sub-002", "sub-002_task-FacePerception_run-1_events.tsv") + with open(events_path, encoding="utf-8", newline="") as fp: + header, rest = fp.read().split("\n", 1) + self.assertIn("\trep_lag\t", header) + with open(events_path, "w", encoding="utf-8", newline="") as fp: + fp.write(header.replace("\trep_lag\t", "\t\t") + "\n" + rest) + + file_paths = io_util.get_file_list( + root_path, extensions=[".tsv", ".json"], exclude_dirs=self.exclude_dirs, name_suffix=["_events"] + ) + events = BidsFileGroup(root_path, file_paths, "events") + group_sidecars = {id(sidecar.contents) for sidecar in events.sidecar_dict.values()} + for data_obj in events.datafile_dict.values(): + if "sub-002" in data_obj.file_path and id(data_obj.sidecar) not in group_sidecars: + data_obj.sidecar.loaded_dict["bad_column"] = {"HED": {"x": "InvalidTagXYZ"}} + # Before the skip path caught the load failure, this raised HedFileError and lost the issues. + issues = events.validate_datafiles(load_schema_version("8.4.0")) + + self.assertEqual([issue["code"] for issue in issues], ["TAG_INVALID"]) + self.assertEqual(issues[0]["ec_sidecarColumnName"], "bad_column") + + def test_trimmed_hed_header_found_without_sidecar(self): + """Discovery trims header cells as BaseInput does: a sidecar-free file headed `HED \\t HED` has HED + columns, so it is validated and its repeated name is TABULAR_INVALID.""" + with tempfile.TemporaryDirectory() as temp_dir: + root_path = os.path.join(temp_dir, "dataset") + shutil.copytree(self.root_path, root_path) + events_path = os.path.join(root_path, "sub-002", "eeg", "sub-002_task-FacePerception_run-1_events.tsv") + with open(events_path, "w", encoding="utf-8", newline="") as fp: + fp.write("onset\tduration\tHED \t HED\n1.0\t0\tRed\tBlue\n") + # Only the .tsv files, so no data file has a sidecar. + file_paths = io_util.get_file_list( + root_path, extensions=[".tsv"], exclude_dirs=self.exclude_dirs, name_suffix=["_events"] + ) + events = BidsFileGroup(root_path, file_paths, "events") + has_hed = [data_obj.file_path for data_obj in events.datafile_dict.values() if data_obj.has_hed] + issues = events.validate_datafiles(load_schema_version("8.4.0")) + + self.assertEqual(len(has_hed), 1) + self.assertTrue(has_hed[0].endswith("sub-002_task-FacePerception_run-1_events.tsv")) + self.assertEqual([issue["code"] for issue in issues], ["TABULAR_INVALID"]) + self.assertEqual(issues[0]["ec_column"], "HED") + def test_summarize(self): events = BidsFileGroup(self.root_path, self.file_paths, "events") info = events.summarize() diff --git a/tests/validator/test_spreadsheet_validator.py b/tests/validator/test_spreadsheet_validator.py index 6cede81ec..d16da20a9 100644 --- a/tests/validator/test_spreadsheet_validator.py +++ b/tests/validator/test_spreadsheet_validator.py @@ -9,6 +9,7 @@ from hed import Sidecar, SpreadsheetInput, TabularInput, load_schema, load_schema_version from hed.errors.error_reporter import ErrorHandler from hed.errors.error_types import ErrorContext, ValidationErrors +from hed.models.column_source import ListColumnSource from hed.validator import SpreadsheetValidator @@ -361,3 +362,58 @@ def test_hed_column_errors_stop_before_assembly(self): self.assertIn(ValidationErrors.TAG_INVALID, codes) self.assertNotIn(ValidationErrors.TEMPORAL_TAG_ERROR, codes) self.assertEqual(error_handler.error_context, []) + + +class TestDuplicateColumns(unittest.TestCase): + """Stage 0: a header that repeats a column name is TABULAR_INVALID and nothing else (spec 3.2.10).""" + + @classmethod + def setUpClass(cls): + cls.schema = load_schema_version("8.4.0") + cls.fixture = os.path.realpath( + os.path.join(os.path.dirname(__file__), "../data/spreadsheet_validator_tests/duplicate_columns.tsv") + ) + + def _validate(self, tabular): + error_handler = ErrorHandler(check_for_warnings=True) + issues = tabular.validate(self.schema, error_handler=error_handler) + self.assertEqual(error_handler.error_context, []) + return issues + + def test_repeated_hed_columns(self): + text = "response\tresponse\tHED\tHED\nx\ty\tInvalidTagXYZ\tRed\n" + issues = self._validate(TabularInput(io.StringIO(text), name="events.tsv")) + # One issue per repeated name, in order of its second appearance; the invalid tag is never checked. + self.assertEqual([issue["code"] for issue in issues], [ValidationErrors.TABULAR_INVALID] * 2) + self.assertEqual([issue[ErrorContext.COLUMN] for issue in issues], ["response", "HED"]) + self.assertEqual([issue[ErrorContext.FILE_NAME] for issue in issues], ["events.tsv"] * 2) + self.assertNotIn(ErrorContext.ROW, issues[0]) + self.assertIn("'response' appears more than once", issues[0]["message"]) + + def test_repeated_column_with_sidecar(self): + # The sidecar has an invalid tag too; the repeated column is reported before the sidecar stage. + sidecar = Sidecar(io.StringIO(json.dumps({"trial": {"HED": {"go": "Red", "stop": "InvalidTagXYZ"}}}))) + issues = self._validate(TabularInput(self.fixture, sidecar=sidecar)) + self.assertEqual(len(issues), 1) + self.assertEqual(issues[0]["code"], ValidationErrors.TABULAR_INVALID) + self.assertEqual(issues[0][ErrorContext.COLUMN], "trial") + self.assertEqual(issues[0][ErrorContext.FILE_NAME], self.fixture) + + def test_empty_name_drops_file_context(self): + tabular = TabularInput(io.StringIO("HED\tHED\nRed\tBlue\n"), name="events.tsv") + issues = tabular.validate(self.schema, name="") + self.assertEqual([issue["code"] for issue in issues], [ValidationErrors.TABULAR_INVALID]) + self.assertNotIn(ErrorContext.FILE_NAME, issues[0]) + + def test_single_columns_not_reported(self): + sidecar = Sidecar(io.StringIO(json.dumps({"trial": {"HED": {"go": "Red", "stop": "Blue"}}}))) + text = "onset\tduration\ttrial\ttrial_type\tHED\n1.0\t0\tgo\tx\tGreen\n" + issues = self._validate(TabularInput(io.StringIO(text), sidecar=sidecar)) + self.assertNotIn(ValidationErrors.TABULAR_INVALID, [issue["code"] for issue in issues]) + + def test_list_column_source_unaffected(self): + source = ListColumnSource({"trial": ["go"], "trial.1": ["stop"], "HED": ["Red"]}) + issues = SpreadsheetValidator(self.schema).validate( + source, error_handler=ErrorHandler(check_for_warnings=False) + ) + self.assertEqual(issues, [])