Fixing duplicate column name issue #1166

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`.

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}."

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"

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:

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 = {}

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