Repository navigation
Show removed keys in colored view when a dict becomes empty - #621
koushik777-ML wants to merge 1 commit into
Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
seperman
left a comment
There was a problem hiding this comment.
Static review: #621 Show removed keys in colored view when a dict becomes empty
Summary: In ColoredView._colorize_json, the dict branch now collects removed keys before the empty-dict early return and returns '{}' only when nothing was removed under that path. This mirrors the list-branch fix already on dev, so a dict that ends up empty now shows its removed keys in red instead of a bare {}. On origin/dev, the dict branch still has if not obj: return '{}' before the removed-key handling, so the bug is real.
Overlap with other PRs
- #573 (merged into
dev) fixed only the list branch for #575. - #578 (closed) contained the same dict-branch change as this PR. The maintainer closed it as already resolved and copied its tests to
dev. However, the copied dict test,test_colored_view_with_empty_dict_shows_removals(tests/test_colored_view.py:542), uses a two-key dict. With the defaultthreshold_to_diff_deeper=0.33,_diff_dictreports that case as a single root-levelvalues_changed, so the test passes without touching the dict-removal branch. - #586 (closed) was list-only.
So this PR supplies the dict half of #578 that never landed. I found no other open PR touching colored_view.py.
Security
No security concerns found in static review. The PR doesn't change how keys and values are escaped or formatted. ANSI codes are still emitted only from the existing constants.
Correctness
deepdiff/colored_view.py:96-98, 107: the logic is correct._get_path_removedis now called once per dict instead of after the item loop, so non-empty dicts do the same amount of work. Empty dicts add one scan ofdiff_paths, which is negligible.- Scope note: with the default
threshold_to_diff_deeper=0.33,_diff_dictemits per-keydictionary_item_removedentries for an emptied dict only when the original dict had a single key (thet_keys_union_len > 1guard indeepdiff/diff.py). With two or more keys the intersection ratio is 0, so the whole dict is reported asvalues_changedand rendered asold -> new, which was already readable. By default, then, this fix affects dicts that had one key, or dicts of any size whenthreshold_to_diff_deeper=0.
Tests
tests/test_colored_view.py:167-211: the three removal tests would fail onorigin/dev, which renders{}. They pass with the change and exercise the dict-removal branch directly, including nesting andCOLORED_COMPACT_VIEW.test_colored_view_empty_dict_without_removals(line 214) is a useful guard for the unchanged{}case.- Suggestion: add a multi-key case with
threshold_to_diff_deeper=0, e.g.DeepDiff({'a': 1, 'b': 2}, {}, view=COLORED_VIEW, threshold_to_diff_deeper=0). That covers the setting where several removed keys reach this branch.
Suggestions
- Issue #575 is still open, and its list case is already fixed on
devby #573. Once this lands, the maintainer may be able to close #575.
AI-assisted static review; the code was not executed.
Problem
While investigating issue #575, I found a related edge case in ColoredView when a dictionary becomes empty after keys are removed.
In this case, the colored view could return before the removed-key handling was processed, resulting in the removed keys not being displayed correctly.
The original list/empty-list behavior reported in #575 is already fixed on the current dev branch, so this PR addresses the related dictionary case.
Changes
Fix the early-return behavior in ColoredView
Ensure removed dictionary keys are shown when the dictionary becomes empty
Add regression tests covering the edge case
Testing
Added regression tests for the dictionary case
Full test suite: 1277 passed, 44 skipped