You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A tabular file whose header repeats a column name (spec 3.2.10) now
gets one TABULAR_INVALID per repeated name, in order of its second
appearance, with ec_filename and ec_column. Validation stops there:
neither the sidecar nor any row is validated or assembled. Issue hed-standard#1166.
BaseInput reads the raw header line before pandas suffixes repeats
and exposes the names as duplicate_columns. Header cells are trimmed
and the trimmed names are the dataframe's column names, so a name
that differs only by whitespace is the same column and a repeat.
The repeated-name check split the raw first line on tabs, so quoted
cells disagreed with the dataframe: trial and "trial" were not a
repeat, and a fully quoted repeat was reported with its quotes. Read
the first record with pandas (header=None, nrows=1) so quotes,
multiline cells, BOM and blank lines are handled as for the data.
validate_datafiles skipped a data file whose merged sidecar had
errors before the header check could run. Load it anyway, and
validate it when its header repeats a column name, so it reports
TABULAR_INVALID and no row is checked against the bad sidecar.
Trim BIDS discovery labels before detecting HED columns
hed/models/base_input.py:511
BIDS discovery needs the same header trimming. For a sidecar-free events file with header HED \t HED, direct TabularInput validation now detects duplicate HED names. However, BidsFileGroup._make_datafile_dict checks untrimmed labels at lines 276-280, leaving has_hed=False, so BIDS validation skips the file without reporting TABULAR_INVALID. Trim the discovery labels before checking for HED columns, and add a sidecar-free BIDS regression for this header.
Buffer text streams when tell() fails after next()
hed/models/base_input.py:537
A seekable text stream can still reject tell(). After next(file) skips a preamble, TextIOWrapper.tell() raises OSError: telling position disabled by next() call, although the remaining TSV is readable. This new call therefore makes loading fail with INVALID_FILE_FORMAT. Fall back to buffering when position reporting fails, and add a regression using TextIOWrapper positioned with next().
_make_datafile_dict compared untrimmed header names, so a file with
no sidecar headed "HED \t HED" had has_hed False and was skipped,
although BaseInput trims names and reports the repeat. Trim them the
same way before looking for the HED column.
TextIOWrapper.tell() raises after next(), so reading the header
record failed for a stream advanced past a preamble, which loaded
before. Read such a stream into memory, as for one that cannot seek.
Since files with an invalid merged sidecar are loaded to look for a
repeated column name, one that cannot be loaded (a blank column name)
raised HedFileError and aborted the run, losing the issues collected.
On that path a load failure now skips the file, as before.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Header normalization breaks existing name-based selections, and cross-format loading changes still need runtime verification.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Normalize column selectors after trimming labels
hed/models/base_input.py:511
Trimming column labels without normalizing the mapper's selectors breaks existing name-based selections, even without duplicate headers. For example, a SpreadsheetInput column named ' HED tags ' selected with tag_columns=[' HED tags '] is renamed to 'HED tags', but ColumnMapper._convert_to_names() still matches the original selector literally. The column is omitted from assembly, and validation reports a missing required column. Prefix-dictionary keys have the same mismatch. Normalize string selectors consistently in mapping and required-column checks, including optional selectors, and add regressions for tag-column and prefix-dictionary selections.
Column names are trimmed when a file is read, but ColumnMapper kept
tag_columns, optional_tag_columns and column_prefix_dictionary keys
as given, so a selector " HED tags " no longer matched its column and
validation reported a missing required column. Trim string selectors
where the mapper stores them.
Column names and top-level sidecar keys are used exactly as read;
the column-name trimming of PR #1470 is removed. Implements
hed-specification 3.2.10 and Appendix B:
- A column name that begins or ends with blank, tab, LF or CR is
TABULAR_INVALID (b); a blank name is TABULAR_INVALID (c). With
repeated names (a), they are reported in the order b, c, a before
the sidecar, and validation stops there.
- The check runs for any ColumnSource, on column_names(), whose
docstring now says the names are exactly as the table holds them.
- A padded top-level sidecar key is SIDECAR_INVALID (d).
- A blank name no longer raises BAD_COLUMN_NAMES at load; the
allow_blank_names switch, check_for_blank_names, the
HED_BLANK_COLUMN warning and the "Unnamed: " prefix rule are
removed. DataFrame labels are taken literally.
- BIDS validation still reports such a file when its merged sidecar
has errors; discovery trims header names only to find HED.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.