Forward ignore_extra to nested checked types in CheckedPMap.create - #328
Open
lntutor wants to merge 1 commit into
Open
Forward ignore_extra to nested checked types in CheckedPMap.create#328lntutor wants to merge 1 commit into
lntutor wants to merge 1 commit into
Conversation
CheckedPMap.create(..., ignore_extra=True) did not pass ignore_extra to the nested checked key/value CheckedType.create calls, so extra fields in a nested checked value (e.g. a PRecord) still raised even though ignore_extra was requested. CheckedPVector/CheckedPSet thread it through correctly via _checked_type_create; CheckedPMap was the one collection that dropped it. Both CheckedType.create and PRecord.create accept ignore_extra, so forwarding it to the key and value create calls is safe. Fixes tobgu#326. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G8b9dEaJtpvydzz4ttM7JH
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.
Fixes #326
Description
CheckedPMap.create(..., ignore_extra=True)does not forwardignore_extrato the nested checked key/valueCheckedType.createcalls. As a result, extra fields in a nested checked value (e.g. aPRecord) still raise even thoughignore_extra=Truewas requested.CheckedPVector/CheckedPSetthreadignore_extracorrectly (via the_checked_type_createhelper);CheckedPMapwas the one collection that dropped it.Root cause
In
CheckedPMap.create(pyrsistent/_checked_types.py), the nestedchecked_key_type.create(key)andchecked_value_type.create(value)calls omittedignore_extra=ignore_extra. The sibling helper_checked_type_create(used byCheckedPVector/CheckedPSet) already forwards it.Fix
Pass
ignore_extra=ignore_extrato both nestedcreatecalls. BothCheckedType.create(the abstract base signature) andPRecord.createacceptignore_extra, so forwarding is safe for every valid key/value type.Added tests
In
tests/checked_map_test.py, mirroring the existingignore_extrapatterns intests/record_test.py:test_create_forwards_ignore_extra_to_value— the exact case from the issue (value type is aPRecord).test_create_without_ignore_extra_still_raises_for_value— the default (ignore_extra=False) still raises.test_create_forwards_ignore_extra_to_nested_checked_value— value type is aCheckedPVectorof records, soignore_extrahas to be forwarded all the way down.The two positive tests fail on the unpatched code and pass with the fix. Full suite:
pytest tests/→ 640 passed, 1 skipped (also verified withPYRSISTENT_NO_C_EXTENSION=1).Note: the fix also forwards
ignore_extraon the key-createpath for symmetry with the value path and with_checked_type_create. That path is not separately unit-tested because a record's source data is adict(unhashable), so it cannot appear as a map key in practice; the meaningful, user-facing path is the value one covered above.