Repository navigation
Conversation
seperman
left a comment
There was a problem hiding this comment.
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_dictbefore a key or attribute name is reported inmatched_paths. The two PRs complement each other:- #638 alone also skips dictionary values of excluded types, so
test_skip_dictionary_value_typewould pass with it. It does not fix the root-object case, and it leaves the item-type check in__searchas it is. - This PR alone still reports keys in
matched_pathswhen their path or value is excluded. - The two merge without a textual conflict (checked with
git merge-tree).
- #638 alone also skips dictionary values of excluded types, so
- 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 oldself.__skip_this(item, parent)checked the type of the search term, not of the object. Two consequences ondev:DeepSearch([1], 1, exclude_types=[list])still reportedroot[0].- Dictionary values and attributes were never type-checked.
__search_dictrelies on__searchfor exclusions, while__search_iterablealready checksthingitself atdeepdiff/search.py:264.
Passing
objmakes the check consistent with__search_iterableand with the documented meaning ofexclude_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. Soexclude_types=[dict]does not skip them by accident. -
User-visible behavior change: on
dev, if the search item's own type was inexclude_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{}ondev. With this change it should reportroot['needle']inmatched_paths, because the value1is 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_dictreports 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, andtest_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]withexclude_types=[list]reportsroot[0]. - With the change, all seven should pass.
- Following
- The existing
test_skip_type_str, where both the object and the item arestr, 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.
|
Added the suggested regression and an Unreleased changelog note in 9071c8e. The new case searches 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 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. |
DeepSearchchecks the search term's type when deciding whether to skip the current object. As a result,DeepSearch([1], 1, exclude_types=[list])still findsroot[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'withexclude_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:
search.py; the existing glob-exclusion branch is outside this search-file run).59acdd9cbb50fdbc30ef7cde2aa6f290ac0a5192search implementation and passes with this fix.RLIMIT_AStest fails before reaching package code; the same failure was reproduced on a separate clean checkout of the exact base.git diff --checkpass.AI assistance: the implementation, regression update, and local validation were prepared with OpenAI Codex.