Skip to content

ENH: Add Support for dict unpacking in csp.node - #730

Open
aadya940 wants to merge 2 commits into
Point72:mainfrom
aadya940:dict-unpacking
Open

ENH: Add Support for dict unpacking in csp.node#730
aadya940 wants to merge 2 commits into
Point72:mainfrom
aadya940:dict-unpacking

Conversation

@aadya940

@aadya940 aadya940 commented Jul 28, 2026

Copy link
Copy Markdown

Reference Issue:

Closes #725

image

Dict Unpacking via ** now works as shown above instead of a CspParseError.

This includes the following:

Single dict Unpacking:

 **{"a1": 1, "a2": 2}

Nested dicts:

**{"a1": 1, "a2": {"a1": 1, "a2": 2}}

Multiple dicts

 (**d1, **d2)

How

Named outputs get resolved at parse time: csp.output(a=1) rewrites to #outp_0 + 1. That can't work for **values, since the keys only exist once the node ticks.

So the parser detects the unpack (ast.keyword with arg=None) and emits a call to a small runtime helper, passing a statically-built {name: proxy} map alongside the unpacked expression. The helper iterates the dict at runtime and ticks each output via the same proxy + value mechanism the generated code already uses. No C++ changes needed, since PyOutputProxy already exposes this through nb_add.

Signed-off-by: Aadya Chinubhai <aadyachinubhai@gmail.com>

@AdamGlustein AdamGlustein left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the contribution, this is great! Just some minor comments to address.

Comment thread csp/impl/wiring/node_parser.py Outdated
Comment thread csp/impl/wiring/node_parser.py Outdated
proxy = outputs_by_name[k]
except KeyError:
raise KeyError(f"unrecognized output '{k}'") from None
proxy + v

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please add a comment here noting we override the ast.Add operator to output a value, elsewise this code looks like it's dead. You may also need to do a ruff noqa to tell ruff to ignore it

Comment thread csp/impl/wiring/node_parser.py Outdated
_CSP_ENGINE_START_TIME_FUNC = "_engine_start_time"
_CSP_ENGINE_END_TIME_FUNC = "_engine_end_time"
_CSP_ENGINE_STATS_FUNC = "_csp_engine_stats"
_CSP_OUTPUT_KWARGS_FUNC = "_csp_output_kwargs"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: you can remove _CSP_OUTPUT_KWARGS_FUNC from the dicts here and just call _csp_output_kwargs in the ast.Call node directly. These dictionaries are meant for functions defined in the C++ code.

Comment thread csp/impl/wiring/node_parser.py Outdated
# A **expr unpack, resolved at runtime, can't statically verify
# which outputs it covers, so assume it may cover all of them.
self._returned_outputs.update(o.name for o in self._signature._outputs if o.name is not None)
nodes.append(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we can try to avoid the function call overhead and generator the for loop AST directly here instead
note you can use @node( debug_print=True ) to dump the generated readable code for debugging

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, I'm working on it, I saw the dict creation per tick is actually more expensive than the jump to the function call, so hopefully minimizing that too, so the same dict can be reused with different runtime values.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

not sure i follow, where would there be an extra dict creation? ** would be replaced with a loop on the variable being expanded, no new dict being created. we should add an isinstance check to your point above ( #730 (comment) )

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

After the inline change it would generate:

while True:
    yield
    if +#inp_0:
        values = {'a': 1, 'b': 2}
        for k, v in values.items():
            {'a': #outp_0, 'b': #outp_1}[k] + v

A dict display is a runtime instruction, Python constructs a fresh dict every time it evaluates { }, so this builds one per iteration. The keys never change though, so instead take the { } outside the while True, right after the first yield, by then PyNode init has already patched the real proxies into the frame locals, so the map captures them once and stays valid for the node's lifetime:

yield
#csp_outmap = {'a': #outp_0, 'b': #outp_1}   # built once with proxies
...
while True:
    yield
    if +#inp_0:
        values = {'a': 1, 'b': 2}
        for #k, #v in values.items():
            #csp_outmap[#k] + #v

Hoping I'm not missing something obvious here!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Right I forgot that you need to lookup the out proxy dynamically but yes of course you would build the map once outside of the generator loop

Signed-off-by: Aadya Chinubhai <aadyachinubhai@gmail.com>
@aadya940

Copy link
Copy Markdown
Author

Works.

(csp-dev) aadya-chinubhai@aadya-laptop:~/Desktop/projects/csp$ python
Python 3.13.14 | packaged by conda-forge | (main, Jun 12 2026, 09:50:25) [GCC 14.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import csp
... from csp import ts
... 
... @csp.node(debug_print=True)
... def foo(x: ts[bool]) -> csp.Outputs(a=ts[int], b=ts[int]):
...     if csp.ticked(x):
...         values = {"a": 1, "b": 2}
...         csp.output(**values)
...         
def foo():
    #node_p = 'node_p', 0
    #inp_0 = 'input_proxy', 0
    #outp_0 = 'output_proxy', 0
    #outp_1 = 'output_proxy', 1
    x = 'input_var', 0
    yield
    del x
    #csp_outmap = {'a': #outp_0, 'b': #outp_1}
    while True:
        yield
        if +#inp_0:
            values = {'a': 1, 'b': 2}
            #csp_vals = values
            if not isinstance(#csp_vals, dict):
                raise TypeError('csp.output argument after ** must be a dict')
            for #csp_k, #csp_v in #csp_vals.items():
                #csp_outmap[#csp_k] + #csp_v

>>> 

Note: This still doesn't cover the following cases:

  • csp.output(values) where values is a dict.
  • csp.output(*args) where args is a list.
  • Duplicate keys tick twice: csp.output(a=1, **{"a": 2}).

Are we looking to add any other operations? I can certainly work on it.

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.

csp.output does not support dict unpacking

3 participants