Skip to content

Commit c66ea45

Browse files
authored
Copy receiver_data in autoimpute to avoid mutating caller's frame (#185)
autoimpute ended with receiver_data[var] = median_imputations[var] which could write through to the caller's original DataFrame depending on whether intermediate pandas operations returned a copy or a view. This is a silent side effect: the user's input frame would gain new columns without any notification. Take a defensive .copy() of receiver_data at the top of autoimpute so the caller's frame is always preserved. The imputed columns are returned exclusively via result.receiver_data. Tests - test_autoimpute_does_not_mutate_caller_receiver asserts that receiver_data passed in remains pristine and the returned frame carries the imputed column.
1 parent 079403d commit c66ea45

3 files changed

Lines changed: 56 additions & 0 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixed `autoimpute` mutating the caller's `receiver_data` (#13). The final `receiver_data[var] = median_imputations[var]` assignment could write through to the caller's original DataFrame depending on whether intermediate pandas operations returned a copy or a view — a subtle side effect that silently added imputed columns to the user's input frame. `autoimpute` now takes a defensive `.copy()` of `receiver_data` at the top of the function so the caller's frame is always preserved, and the imputed columns are returned exclusively through `result.receiver_data`.

microimpute/comparisons/autoimpute.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,16 @@ def autoimpute(
441441
main_progress = tqdm(total=5, desc="AutoImputation progress")
442442
main_progress.set_description("Input validation")
443443

444+
# Defensive copy so that the caller's receiver_data is never
445+
# mutated. Previously the final assignment
446+
# ``receiver_data[var] = median_imputations[var]`` would write
447+
# back through any local binding, and whether the user's frame
448+
# was affected depended on whether the intermediate ``drop`` call
449+
# returned a copy or a view (#13). Copying up-front makes this
450+
# explicit and eliminates the side effect regardless of later
451+
# pandas internals.
452+
receiver_data = receiver_data.copy()
453+
444454
# Use provided quantiles or defaults
445455
quantiles = imputation_quantiles if imputation_quantiles else QUANTILES
446456

tests/test_autoimpute.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,51 @@ def test_autoimpute_invalid_model_specification() -> None:
403403
)
404404

405405

406+
def test_autoimpute_does_not_mutate_caller_receiver() -> None:
407+
"""Regression test for #13: autoimpute must not mutate the caller's
408+
receiver_data. Previously the final ``receiver_data[var] = ...``
409+
assignment could write back to the caller's frame depending on
410+
pandas copy/view semantics during preprocessing, silently adding
411+
new columns to the user's DataFrame.
412+
"""
413+
from microimpute.models import OLS
414+
415+
np.random.seed(0)
416+
donor = pd.DataFrame(
417+
{
418+
"x1": np.random.randn(80),
419+
"x2": np.random.randn(80),
420+
"y": np.random.randn(80),
421+
}
422+
)
423+
receiver = pd.DataFrame(
424+
{
425+
"x1": np.random.randn(30),
426+
"x2": np.random.randn(30),
427+
}
428+
)
429+
snapshot = receiver.copy()
430+
431+
result = autoimpute(
432+
donor_data=donor,
433+
receiver_data=receiver,
434+
predictors=["x1", "x2"],
435+
imputed_variables=["y"],
436+
models=[OLS],
437+
log_level="WARNING",
438+
)
439+
440+
# Caller's frame must be untouched.
441+
assert "y" not in receiver.columns, (
442+
"autoimpute silently added the imputed column to the caller's "
443+
"receiver_data DataFrame"
444+
)
445+
pd.testing.assert_frame_equal(receiver, snapshot)
446+
447+
# The returned frame carries the imputed values.
448+
assert "y" in result.receiver_data.columns
449+
450+
406451
# === Performance Tests ===
407452

408453

0 commit comments

Comments
 (0)