Skip to content

Preserve box options in BoxList.__deepcopy__#319

Open
chuenchen309 wants to merge 1 commit into
cdgriffith:masterfrom
chuenchen309:fix/boxlist-deepcopy-preserve-options
Open

Preserve box options in BoxList.__deepcopy__#319
chuenchen309 wants to merge 1 commit into
cdgriffith:masterfrom
chuenchen309:fix/boxlist-deepcopy-preserve-options

Conversation

@chuenchen309

Copy link
Copy Markdown

BoxList.__deepcopy__ builds the copy as self.__class__() with no box_options, so copy.deepcopy() silently drops every option:

import copy
from box import BoxList
d = copy.deepcopy(BoxList([1, 2, 3], frozen_box=True))
d.box_options.get("frozen_box")   # None  — frozen_box lost
d.append(4)                        # succeeds -> [1, 2, 3, 4]  — immutability broken (and now unhashable)

It's a three-way inconsistency: BoxList.__copy__ preserves options, Box.__deepcopy__ preserves options (and re-applies frozen), but BoxList.__deepcopy__ does neither. It's also reachable indirectly — copy.deepcopy of a Box that contains a nested BoxList loses that BoxList's options.

Fix (box/box_list.py): mirror Box.__deepcopy__ — build the copy unfrozen (so append works while populating), register it in memo before recursing (preserves the existing self-reference/cycle safety), populate, then re-freeze via __init__.

Verification (re-runnable from the diff): added test_deepcopy_preserves_box_options — it checks copy.deepcopy preserves frozen_box/box_dots/default_box/box_intact_types (parity with copy.copy), and that a deepcopied frozen BoxList still raises on append and hashes equal. It fails on the current tree and passes with the fix. test/test_box_list.py goes from 18 to 19 passing with no new failures (the 2 pre-existing failures are test_toon_*, an optional toon dep not installed — identical before and after).


Disclosure: This PR was authored by an AI coding agent (Claude Code) running on this account: the AI found the bug, ran the repro, wrote the test, and wrote this description. The human account holder reviews every change and is accountable for it. The verification above is real and re-runnable from the diff. If this isn't the kind of contribution you want, say so and I'll close it.

`BoxList.__deepcopy__` built the copy as `self.__class__()` with no
`box_options`, so `copy.deepcopy()` silently dropped every option
(`frozen_box`, `box_dots`, `default_box`, `box_intact_types`, …). A frozen
BoxList became mutable (and unhashable) after a deepcopy. `BoxList.__copy__`
and `Box.__deepcopy__` both preserve options; `BoxList.__deepcopy__` was the
outlier.

Mirror `Box.__deepcopy__`: build unfrozen, register in memo before recursing
(keeps cycle-safety), populate, then re-freeze via `__init__`.
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.

1 participant