Support dict unpacking in dict literals in the local Python executor - #2553
Support dict unpacking in dict literals in the local Python executor#2553himanshu748 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7eef5d78c0
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for key_node, value_node in zip(expression.keys, expression.values): | ||
| if key_node is None: | ||
| value = evaluate_ast(value_node, *common_params) | ||
| if not isinstance(value, Mapping): |
There was a problem hiding this comment.
Allow mapping-protocol objects in dict unpacking
When ** is used with objects that implement the normal mapping protocol but do not inherit/register with collections.abc.Mapping (for example a user-defined class with keys() and __getitem__(), or third-party mapping-like objects), CPython accepts {**obj} but this check rejects it with 'MyMap' object is not a mapping. Since the executor supports user-defined classes and the change is intended to match Python dict-unpacking semantics, this still breaks valid Python code in those contexts; consider trying result.update(value) and translating its TypeError instead of pre-filtering by the ABC.
Useful? React with 👍 / 👎.
Fixes #2552.
What happens today
Dict unpacking inside a dict literal, one of the most common patterns in model-generated code, crashes the local executor with an error that points nowhere:
{**{"a": 1}, "b": 2} # InterpreterError: NoneType is not supported.The
ast.Dictbranch inevaluate_astevaluates every element ofexpression.keys, but a**mappingentry hasNoneas its AST key, soNonereachesevaluate_astand fails with the unrelated message. The model gets no signal that**is the problem and typically retries the same syntax until the run dies.The fix
Evaluate the dict literal pairwise: a
Nonekey marks a**mappingentry whose evaluated value is merged withdict.update, matching CPython semantics (in-order merge, later keys win). Unpacking a non-mapping now raises the same message CPython gives:{**[1, 2]} # InterpreterError: 'list' object is not a mappingKey and value evaluation order for normal entries is preserved (key before value).
Tests
7 new tests: merge with literal keys, double unpack, later-key override in both directions, unpacking a stored variable, empty unpack and the non-mapping error. All fail on current main and pass on this branch. Full test file: 404 passed, 2 skipped. Ruff check and format are clean.