Skip to content

Apply search type exclusions to the current object - #629

Open
jakezwang wants to merge 2 commits into
qlustered:devfrom
jakezwang:fix/search-excluded-object-types
Open

jakezwang wants to merge 2 commits into
qlustered:devfrom
jakezwang:fix/search-excluded-object-types

Conversation

@jakezwang

@jakezwang jakezwang commented Sep 18, 2026 •

Copy link
Copy Markdown

DeepSearch checks the search term's type when deciding whether to skip the current object. As a result, DeepSearch([1], 1, exclude_types=[list]) still finds root[0], and excluded objects nested in dictionaries are searched too.

Pass the current object to the existing exclusion check. Add regressions for excluded root containers and dictionary values, including custom objects and an unexcluded sibling. Also cover an excluded search-term type: searching {'needle': 1, 'value': 'needle'} for 'needle' with exclude_types=[str] reports the path for the integer value and excludes the string value. Add an Unreleased changelog note explaining the behavior change.

Validation on Python 3.14.7 / macOS ARM64:

  • All 70 search tests pass (99% statement coverage of search.py; the existing glob-exclusion branch is outside this search-file run).
  • The newly requested guard test fails with the exact unpatched base 59acdd9cbb50fdbc30ef7cde2aa6f290ac0a5192 search implementation and passes with this fix.
  • Full suite including slow tests: 1,316 passed and 8 skipped. One existing macOS RLIMIT_AS test fails before reaching package code; the same failure was reproduced on a separate clean checkout of the exact base.
  • Strict flake8 checks on the changed test and git diff --check pass.

AI assistance: the implementation, regression update, and local validation were prepared with OpenAI Codex.

@seperman seperman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static review: #629 Apply search type exclusions to the current object

Summary: DeepSearch.__search now passes the current object to __skip_this instead of the search item, so exclude_types applies to the object being visited. This fixes the reported cases: excluded root containers and excluded dictionary or attribute values were still being searched.

Overlap with other PRs

  • #638 (open) also changes exclusion handling in deepdiff/search.py, but at a different step. It adds __skip_this(obj_child, new_parent) in __search_dict before a key or attribute name is reported in matched_paths. The two PRs complement each other:
    • #638 alone also skips dictionary values of excluded types, so test_skip_dictionary_value_type would pass with it. It does not fix the root-object case, and it leaves the item-type check in __search as it is.
    • This PR alone still reports keys in matched_paths when their path or value is excluded.
    • The two merge without a textual conflict (checked with git merge-tree).
  • Nothing equivalent is on dev.

Security

No security concerns found in static review. The change is one line of logic plus tests, with no new imports, I/O, or config changes.

Correctness

  • deepdiff/search.py:322: the old self.__skip_this(item, parent) checked the type of the search term, not of the object. Two consequences on dev:

    • DeepSearch([1], 1, exclude_types=[list]) still reported root[0].
    • Dictionary values and attributes were never type-checked. __search_dict relies on __search for exclusions, while __search_iterable already checks thing itself at deepdiff/search.py:264.

    Passing obj makes the check consistent with __search_iterable and with the documented meaning of exclude_types ("List of object types to exclude from the report").

  • Custom objects and namedtuples are turned into a dict and passed straight to __search_dict (deepdiff/search.py:173-192), not through __search. So exclude_types=[dict] does not skip them by accident.

  • User-visible behavior change: on dev, if the search item's own type was in exclude_types, the whole search was skipped and always returned {}. Now only objects of that type are skipped. For example, DeepSearch({'needle': 1}, 'needle', exclude_types=[str]) returned {} on dev. With this change it should report root['needle'] in matched_paths, because the value 1 is not excluded. This looks like the intended behavior, but it may be worth a changelog note.

  • Not covered here: a key or attribute name whose value is excluded (by path or by type) is still reported in matched_paths. __search_dict reports the path match (deepdiff/search.py:242-248) before it calls __search. #638 addresses this.

Tests

  • The new tests are test_skip_root_object_type (tests/test_search.py:264), which covers root list, tuple, dict and custom object, and test_skip_dictionary_value_type (tests/test_search.py:268), which covers excluded dictionary values next to a kept sibling.
    • Following dev's code, all seven cases should fail there. For example, [item] with exclude_types=[list] reports root[0].
    • With the change, all seven should pass.
  • The existing test_skip_type_str, where both the object and the item are str, still passes under the new check.

Suggestions

  • Consider a regression test for the behavior change above: the item's type is excluded, and the match is in an object of a non-excluded type. That would pin down the new behavior.
  • Consider landing this together with #638, so that exclusions apply both to value matches and to key or attribute-name matches.

AI-assisted static review; the code was not executed.

@jakezwang

Copy link
Copy Markdown
Author

Added the suggested regression and an Unreleased changelog note in 9071c8e.

The new case searches {'needle': 1, 'value': 'needle'} for 'needle' with exclude_types=[str]. It keeps root['needle'] in matched_paths because its value is an integer, while still excluding the string value. The test fails with the unpatched base's search implementation and passes with this fix.

On Python 3.14.7 / macOS ARM64, all 70 search tests pass. The full slow suite reports 1,316 passed and 8 skipped, with the existing test_restricted_unpickler_memory_exhaustion_cve failure at macOS resource.setrlimit(RLIMIT_AS, ...); I reproduced that same failure on a separate clean checkout of base 59acdd9cbb50fdbc30ef7cde2aa6f290ac0a5192. Strict flake8 on the changed test and git diff --check also pass.

The key/attribute reporting changes in #638 remain separate, as described in your review; this update only adds the requested guard and changelog note.

AI assistance: this follow-up and its test execution were prepared with OpenAI Codex.

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.

2 participants