Skip to content

gh-160929: Preserve ChainMap lookup values in reverse unions - #160932

Open
JoaoMarcos44 wants to merge 3 commits into
python:mainfrom
JoaoMarcos44:fix-chainmap-reverse-union
Open

JoaoMarcos44 wants to merge 3 commits into
python:mainfrom
JoaoMarcos44:fix-chainmap-reverse-union

Conversation

@JoaoMarcos44

@JoaoMarcos44 JoaoMarcos44 commented Oct 29, 2026 •

Copy link
Copy Markdown

Why

dict | ChainMap can replace a value exposed by the right-hand mapping with a deeper map's stored value. For example, ChainMap(Counter(), {'x': 5})['x'] is 0, but its reverse union returned 5.

What changed

  • Update the result from the ChainMap view so its lookup semantics determine the values.
  • Cover Counter and defaultdict, including collisions, key order, and the original maps' state.
  • Add a NEWS entry.

Scope

The change is limited to reverse union and its regression tests.

Tradeoffs

The union now performs per-key chain lookups, as dict(chain) does. Long disjoint chains can require quadratic lookup work. Default factories can run and insert missing keys, as they do when reading the same ChainMap directly.

Verification

  • Built native CPython 3.16.0a1+dev on Ubuntu 24.04. Both regression tests fail against the original module and pass with the fix; the minimal reproduction changes from the wrong 5 to the expected 0.
  • Ran test_collections test_userdict test_userlist test_userstring test_numeric_tower with ten hash seeds. Each pass reports 286 tests, 284 passes, two skips, and zero failures or errors. Both new tests are present and pass in every run. Native validation and results.
  • Repository lint, make patchcheck, and git diff --check pass. Lint result. Contribution checklist result.

Illustrated explanation

The first Counter() layer returns 0 for the missing key. The old reverse union copied the deeper stored 5; the fix preserves the value returned by the ChainMap view.

Illustrated ChainMap reverse union: the empty Counter returns zero for missing x; the old merge copies five from a deeper layer, while the corrected union reads through the view and preserves zero.

Accessible text explanation and validation notes. The illustration is supplemental; it is hosted outside this source patch.

Fixes #160929.

AI tools assisted the investigation, patch, tests, and supplemental illustration. I have personally reviewed the AI-assisted material in this PR.

@python-cla-bot

python-cla-bot Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-29T21:50:57.754299Z
We couldn’t complete this review. Comment @codex review to try again.
aa59123 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@rhettinger rhettinger self-assigned this Oct 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ChainMap reverse union bypasses missing-key lookup semantics

2 participants

⌂ Proxy