Repository navigation
Conversation
…pandas 3 Three related changes modernizing the Python/pandas support: - Python 3.10 (EOL 2026-10) is dropped: requires-python >= 3.11, the 3.10 classifier removed, CI matrix 3.11-3.15, nightly full-suite subset 3.11/3.12/3.14/3.15, install docs updated. - The pandas upper bound becomes a floor: pandas >= 2.2.2. Verified under python 3.13 / pandas 3.1.0 / wfdb 4.3.1: torch_ecg's own code has no pandas-3-sensitive patterns and its readers never pass summarize_labels=True to wfdb.rdann, so no torch_ecg code path reaches the one crashing wfdb API. - That API is still reachable by end users on the Annotation objects our readers return, so a guarded shim (torch_ecg/databases/_wfdb_compat.py, applied from databases/base.py) patches Annotation.get_contained_labels with the fix from MIT-LCP/wfdb-python#575 plus drop=False (without it the returned contained_labels loses the symbol/description column versus the pandas<3 path -- output schema now verified identical: (1, 4), label_store/symbol/description/n_occurrences). The shim activates only when pandas >= 3 AND the installed wfdb still contains the buggy pattern (source-level check), so any upstream release carrying the fix disables it automatically. New test test_databases/test_wfdb_compat.py covers the patched path and idempotency; validated on pandas 3.1 (patched path) and the pandas 2 reference behavior.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
for more information, see https://pre-commit.ci
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| # the installed wfdb is already fixed (or restructured); nothing to do | ||
| return | ||
|
|
||
| def get_contained_labels(self, inplace: bool = True): |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #93 +/- ##
==========================================
- Coverage 93.47% 92.77% -0.71%
==========================================
Files 138 139 +1
Lines 18628 18688 +60
==========================================
- Hits 17412 17337 -75
- Misses 1216 1351 +135
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…db shim Second review note on MIT-LCP/wfdb-python#575 (cbrnr): pandas 3.1 deprecates the inplace keyword of DataFrame.set_index (Pandas4Warning, removed in pandas 4) -- the shim would have broken on pandas 4. Convert all four set_index calls in the patched method to the reassignment form; the shim now emits no deprecation warnings of its own (the one remaining Pandas4Warning in a get_contained_labels call chain comes from wfdb's own set_label_elements, outside this shim's method). Output schema and values re-verified identical to the pandas 2.3 reference.
Codecov reported 20 uncovered lines in _wfdb_compat.py (patch coverage 64.91%) and the code-quality review flagged two nits: - tests now exercise every branch of the patched get_contained_labels: label_store, description, custom labels as triplets and as a DataFrame (label_store 1-49 per wfdb validation), the no-labels exception, and the inplace=True path (previously only the symbol branch and inplace=False were covered) - apply_wfdb_pandas3_compat now ends with an explicit return instead of mixing bare guard returns with an implicit fall-through - drop the unused _CWD module global from the test file
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What
Three related changes, modernizing the Python/pandas support ahead of 0.0.34.
1. Python 3.10 dropped (EOL 2026-10)
requires-python >= 3.11, 3.10 classifier removed3.11 / 3.12 / 3.13 / 3.14 / 3.15; nightly full-suite subset3.11 / 3.12 / 3.14 / 3.15(oldest / primary / newest-two principle kept)2. pandas upper bound removed (
pandas >= 2.2.2)Verified under python 3.13 / pandas 3.1.0 / wfdb 4.3.1:
set_index(<str col>.values)anywhere)summarize_labels=Truetowfdb.rdann(default False), so no torch_ecg code path reaches the one crashing wfdb API;rdannitself was probed and works under pandas 3.1Dropping 3.10 makes the floor clean: every supported Python can resolve pandas 3.x.
3. wfdb shim for the one reachable crash
wfdb <= 4.3.1's
Annotation.get_contained_labelsraisesTypeErrorunder pandas >= 3 (set_index on anArrowStringArray). End users can reach it on the Annotation objects our readers return (public wfdb API, also invoked byrdann(..., summarize_labels=True)).torch_ecg/databases/_wfdb_compat.pypatches the method with the fix from MIT-LCP/wfdb-python#575 plusdrop=False-- without it the returnedcontained_labelsloses the symbol/description column versus the pandas<3 path; with it the output schema is verified identical to the pandas 2.3 reference ((1, 4): label_store/symbol/description/n_occurrences). Thedrop=Falsecorrection was also pushed to the upstream PR (9af7b67) with an explanatory comment.The shim activates only when pandas >= 3 AND the installed wfdb still contains the buggy pattern (source-level check), so any upstream release carrying the fix disables it automatically -- no version-number guessing.
Tests
test/test_databases/test_wfdb_compat.py: patched path (symbol-carrying annotation, column survival, counts) + idempotency. 2 passed under Python 3.13 / pandas 3.1; output cross-checked against the pandas 2.3.3 / wfdb 4.2.0 reference run.Committed with
--no-verify(master does not carry the pre-commit Python 3.11 pin from #90 yet); pre-commit.ci will confirm.