fix: preserve dict subclass types in _deepcopy_with_exceptions - #13094
Open
inchang-ing wants to merge 1 commit into
Open
inchang-ing wants to merge 1 commit into
inchang-ing wants to merge 1 commit into
Conversation
The dict branch rebuilt every dict with a dict comprehension, so `defaultdict`, `OrderedDict` and `Counter` values passed to a pipeline came back as plain `dict`. `Pipeline.run` copies its inputs with this helper, so a component received a different type than the caller passed in, and a `defaultdict` lost its `default_factory` in the process. Rebuild the container as its own type (copy, clear and refill with deep-copied values) instead of collapsing it to a plain `dict`, matching the existing `type(obj)` handling for lists/tuples/sets and namedtuples.
Contributor
|
@inchang-ing is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
Author
|
I have read the CLA Document and I hereby sign the CLA |
5 of 6 tasks
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.
Related Issues
Proposed Changes:
_deepcopy_with_exceptionsrebuilt everydictwith a dict comprehension, so anydictsubclass came back as a plaindict:A
defaultdictlost itsdefault_factory, anOrderedDictlost its ordering guarantee, and aCounterwas demoted todict. Thelist/tuple/setbranch right above already preserves the concrete type viatype(obj)(...), and thenamedtuplespecial case (see the release notefix-deepcopy-namedtuple-*) does the same, so dict subclasses were the remaining gap. BecausePipeline.runcopies its inputs with this helper, a component could receive a different type than the caller passed in.The dict branch now rebuilds the container as its own type instead of a plain
dict:A plain
copyis used rather thantype(obj)(<iterable>)because dict subclasses have incompatible constructors (defaultdicttakesdefault_factoryas its first positional argument), so a generic iterable-based rebuild is not safe. Copying the (empty) container preserves the subclass and all of its configuration — includingdefaultdict.default_factory— and refilling it with deep-copied values keeps the deep-copy semantics identical to the plain-dictpath.How did you test it?
Added
TestDeepcopyWithFallbackcases intest/core/pipeline/test_utils.py:defaultdict,OrderedDictandCounterasserting the type is preserved and the values are deep-copied;defaultdictkeeps itsdefault_factory(a missing key still triggers the factory instead of raisingKeyError).Verified the tests fail on
main(the type assertion seesdictinstead of the subclass) and pass with the change.Notes for the reviewer
copy(obj)on a plaindictreturns a plaindict, so existing behavior for the common case is unchanged.releasenotes/notes/fix-deepcopy-dict-subclass-*.yaml), matching the earlier namedtuple fix.Checklist
fix:and added!in case the PR includes breaking changes.This PR was fully generated with an AI assistant. I have reviewed the changes and run the relevant tests.