Skip to content

fix: handle read-only arrays in DunedinPACE quantile normalization (#195)#206

Open
IsraelAfangideh wants to merge 2 commits into
bio-learn:masterfrom
IsraelAfangideh:fix/dunedin-pace-readonly-array
Open

fix: handle read-only arrays in DunedinPACE quantile normalization (#195)#206
IsraelAfangideh wants to merge 2 commits into
bio-learn:masterfrom
IsraelAfangideh:fix/dunedin-pace-readonly-array

Conversation

@IsraelAfangideh

Copy link
Copy Markdown

Summary

Fixes #195.

DunedinPACE.predict raises ValueError: assignment destination is read-only during quantile normalization:

File ".../biolearn/dunedin_pace.py", line 61, in quantile_normalize_using_target
    column_data[has_decimal_above_0_4] = 0.5 * (
ValueError: assignment destination is read-only

Root cause

quantile_normalize_using_target mutates its input array in place. The array is passed in as dataframe_transposed.values. In pandas >= 3.0 (and with newer numpy), DataFrame.values returns a read-only array by default (copy-on-write), so the in-place writes fail.

This is why the reporter saw the same data work under an older Python/pandas stack but break on a newer one — it was never the Python version itself, but the pandas/numpy versions that ship with it.

Fix

Operate on a writable float copy at the top of the function:

data = np.array(data, dtype=float)

This resolves the crash and, as a bonus, stops the function from mutating the caller's array (it now returns a fresh array as the name implies).

Testing

  • Reproduced the exact ValueError on the default pandas 3.0 path before the change.
  • Added regression tests in biolearn/test/test_dunedin_pace.py covering: read-only input, no mutation of the caller's array, and correct mapping onto the target distribution.
  • Verified end-to-end against test_model.py::test_dunedin_pace_normalization: on master it fails at the read-only line; with this change it gets past normalization.

Note

While verifying end-to-end on a pandas 3.0 environment, I noticed a separate, pre-existing failure further downstream in the DunedinPACE test path (a KeyError from an integer RangeIndex where CpG labels are expected). It is unrelated to this read-only fix and is masked on master because the read-only error aborts earlier. I've kept it out of this PR and am happy to open a separate issue/PR for it.

quantile_normalize_using_target mutated its input array in place. When
the array originates from DataFrame.values it can be read-only (pandas
copy-on-write, default in pandas >= 3.0), raising "assignment
destination is read-only" during DunedinPACE.predict. This also explains
why the same data worked under older pandas/numpy but broke on newer ones.

Operate on a writable float copy instead, which fixes the crash and
avoids mutating the caller's array.

Adds regression tests covering the read-only input, no-mutation, and
target-distribution mapping behavior.

Fixes bio-learn#195

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread biolearn/dunedin_pace.py Outdated
"""
Apply quantile normalization on data using target values.
"""
# Copy to a writable float array. Inputs coming from ``DataFrame.values``

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this comment could be condensed significantly

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call — condensed it to two lines, keeping just the why (read-only DataFrame.values under pandas 3.0 copy-on-write breaking the in-place writes). Pushed in bf51b76. Thanks for the review!

Tighten the explanatory comment on the writable-copy line from four
lines to two, keeping the essential "why" (read-only DataFrame.values
under pandas>=3.0 copy-on-write breaks the in-place updates below).
Comment-only; no behavior change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Issue with DunedinPACE model

2 participants