Updated to support Pandas 3 - addresses issue #1291#1292
Updated to support Pandas 3 - addresses issue #1291#1292VisLab merged 3 commits intohed-standard:mainfrom
Conversation
|
Claude code review could not run. The request to the Claude API failed (possible causes: billing limit reached, token expired, or service unavailable). Review this PR manually. |
There was a problem hiding this comment.
Pull request overview
Updates hedtools to be compatible with pandas 3.x (copy-on-write and dtype/NA behavior changes), while also adding small usability enhancements to BIDS sidecar extraction and BIDS file grouping.
Changes:
- Relax pandas upper bound to
<4.0.0and apply targeted pandas 3 compatibility fixes (CoW-safe assignment, drop API change, NaN/StringDtype handling). - Add
--filter/-flfilename filtering tohedpy extract bids-sidecar/hed_extract_bids_sidecar. - Add
BidsFileGroup.get_task_names()plus new/expanded regression tests (including a dedicated pandas3 compat test suite).
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/tools/bids/test_bids_file_group.py | Adds tests for new BidsFileGroup.get_task_names() behavior. |
| tests/test_pandas3_compat.py | New regression suite covering pandas 3 breaking changes across multiple modules. |
| tests/scripts/test_extract_bids_sidecar.py | Adds tests for the new --filter/-fl CLI behavior. |
| pyproject.toml | Relaxes pandas constraint to allow pandas 3.x. |
| hed/tools/util/data_util.py | Removes conflicting axis= usage in DataFrame.drop(columns=...). |
| hed/tools/bids/bids_file_group.py | Adds get_task_names() helper for extracting unique task entities from filenames. |
| hed/scripts/hed_extract_bids_sidecar.py | Implements filename filtering and switches to explicit TabularSummary.update(file_list). |
| hed/schema/schema_io/hed_id_util.py | Hardens hedId handling for NaN/non-string values and float-typed all-NaN columns. |
| hed/schema/schema_io/df2schema.py | Avoids string operations on NaN by guarding with isinstance(..., str). |
| hed/schema/schema_io/df_util.py | Updates merge fill logic to handle StringDtype/non-object columns correctly. |
| hed/models/df_util.py | Makes CoW-safe assignment in shrink_defs and uses .iloc/dtype preservation in _filter_by_index_list. |
| hed/cli/cli.py | Exposes --filter/-fl option on hedpy extract bids-sidecar. |
| CHANGELOG.md | Documents pandas 3 compatibility work and the new extraction/task helper features. |
| # Combine default skip columns with user-specified ones | ||
| # Default skips: onset, duration, sample (timing/indexing columns) | ||
| default_skip = ["onset", "duration", "sample"] | ||
| skip_cols = default_skip.copy() | ||
| if args.skip_columns: | ||
| skip_cols.extend(args.skip_columns) | ||
|
|
||
| logger.debug(f"Skip columns: {skip_cols}") |
There was a problem hiding this comment.
skip_cols is currently built by starting with default_skip = ['onset','duration','sample'] and then unconditionally extending it with args.skip_columns. Since the argparse option already defaults args.skip_columns to the same list, the default path produces duplicated entries (and if the user supplies --skip-columns, defaults are re-added in a way that’s a bit unclear). Consider normalizing this by either (a) making the argparse default []/None and then unioning with default_skip, or (b) deduplicating via an ordered set/dict.fromkeys before passing to TabularSummary.
There was a problem hiding this comment.
We fixed by adding deduplication in tabular_summary for skip_cols. We did not change the user interface.
|
Claude code review could not run. The request to the Claude API failed (possible causes: billing limit reached, token expired, or service unavailable). Review this PR manually. |
|
Claude code review could not run. The request to the Claude API failed (possible causes: billing limit reached, token expired, or service unavailable). Review this PR manually. |
No description provided.