Skip to content

Forward ignore_extra to nested key/value types in CheckedPMap.create (fixes #326)#327

Open
Sanjays2402 wants to merge 1 commit into
tobgu:masterfrom
Sanjays2402:fix/checkedpmap-ignore-extra
Open

Forward ignore_extra to nested key/value types in CheckedPMap.create (fixes #326)#327
Sanjays2402 wants to merge 1 commit into
tobgu:masterfrom
Sanjays2402:fix/checkedpmap-ignore-extra

Conversation

@Sanjays2402

Copy link
Copy Markdown

Summary

CheckedPMap.create(source_data, ignore_extra=True) does not forward ignore_extra to the nested checked key/value .create() calls, so an extra field in a nested checked value (or key) still raises AttributeError even though ignore_extra=True was explicitly requested.

Fixes #326.

Reproduction (before)

from pyrsistent import CheckedPMap, PRecord, field

class Rec(PRecord):
    a = field()

class M(CheckedPMap):
    __key_type__ = str
    __value_type__ = Rec

M.create({"k": {"a": 1, "extra": 2}}, ignore_extra=True)
# AttributeError: 'extra' is not among the specified fields for Rec

The same call succeeds for the sibling collections CheckedPVector / CheckedPSet, which honor ignore_extra:

class V(CheckedPVector):
    __type__ = Rec

V.create([{"a": 1, "extra": 2}], ignore_extra=True)   # V([Rec(a=1)])  -- works

Root cause

CheckedPMap.create (in pyrsistent/_checked_types.py) recursively converts nested data via checked_key_type.create(key) / checked_value_type.create(value) but omits ignore_extra:

if checked_key_type or checked_value_type:
    return cls(dict((checked_key_type.create(key) if ... else key,
                     checked_value_type.create(value) if ... else value)
                    for key, value in source_data.items()))

The sibling helper _checked_type_create (used by CheckedPVector / CheckedPSet via create = classmethod(_checked_type_create)) already threads it through:

return cls([checked_type.create(data, ignore_extra=ignore_extra) ...])

So CheckedPMap was the only checked collection dropping the flag.

Fix

Pass ignore_extra=ignore_extra to both the key and value create() calls, mirroring _checked_type_create. Behavior with the default ignore_extra=False is unchanged — extra fields still raise.

-            return cls(dict((checked_key_type.create(key) if checked_key_type and not any(isinstance(key, t) for t in key_types) else key,
-                             checked_value_type.create(value) if checked_value_type and not any(isinstance(value, t) for t in value_types) else value)
+            return cls(dict((checked_key_type.create(key, ignore_extra=ignore_extra) if checked_key_type and not any(isinstance(key, t) for t in key_types) else key,
+                             checked_value_type.create(value, ignore_extra=ignore_extra) if checked_value_type and not any(isinstance(value, t) for t in value_types) else value)
                             for key, value in source_data.items()))

After

M.create({"k": {"a": 1, "extra": 2}}, ignore_extra=True)
# M({'k': Rec(a=1)})            <- extra dropped, as requested

M.create({"k": {"a": 1, "extra": 2}})
# AttributeError: 'extra' is not among the specified fields for Rec   <- default still raises

Tests

Added three regression tests to tests/checked_map_test.py:

  • test_create_ignore_extra_forwarded_to_value_type — extra field in a nested checked value.
  • test_create_ignore_extra_forwarded_to_key_type — extra field in a nested checked key (using a hashable pmap raw key that is converted through KeyRecord.create, so the key .create() path is actually exercised).
  • test_create_ignore_extra_false_still_raises — the default ignore_extra=False still raises AttributeError.

The two ignore_extra=True tests were verified to fail without the source change and pass with it (source-file stash proof):

# without the fix:
FAILED tests/checked_map_test.py::test_create_ignore_extra_forwarded_to_value_type
FAILED tests/checked_map_test.py::test_create_ignore_extra_forwarded_to_key_type
2 failed, 1 passed

# with the fix:
3 passed

Full suite: 640 passed, 1 skipped (no regressions).

Diff is +42/−3 (2 logical source lines + 3 tests).

CheckedPMap.create(source_data, ignore_extra=True) did not pass
ignore_extra down to the nested checked key/value type .create() calls,
so an extra field in a nested checked value (or key) still raised
AttributeError even though ignore_extra=True was requested:

    class Rec(PRecord):
        a = field()
    class M(CheckedPMap):
        __key_type__ = str
        __value_type__ = Rec
    M.create({'k': {'a': 1, 'extra': 2}}, ignore_extra=True)
    # AttributeError: 'extra' is not among the specified fields for Rec

Root cause: in CheckedPMap.create the recursive
checked_key_type.create(key) / checked_value_type.create(value) calls
omit ignore_extra. The sibling helper _checked_type_create used by
CheckedPVector / CheckedPSet already threads ignore_extra through, so
CheckedPMap was the only collection dropping it.

Fix: pass ignore_extra=ignore_extra to both the key and value
create() calls, mirroring _checked_type_create. Behavior with the
default ignore_extra=False is unchanged (extra fields still raise).

Added regression tests covering ignore_extra threading to the value
type and to the key type (via a hashable pmap raw key that is converted
through KeyRecord.create), plus a test asserting the default still
raises. The two ignore_extra=True tests fail without this change and
pass with it.

Fixes tobgu#326
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.

CheckedPMap.create does not forward ignore_extra to nested CheckedType keys/values

1 participant