Support stretchy delays in box
#13899
Conversation
Pull Request Test Coverage Report for Build 15470983441Details
💛 - Coveralls |
|
I'm sure you're probably quite busy at the moment, but depending on when 2.0 is going out, I can review this tomorrow if you want to rebase it. |
Mostly this just needed to re-use the existing handling from within `Delay`, and to teach the control-flow builders to look in another special place (`BoxOp.duration`) for potential `Expr` instances to close over.
|
One or more of the following people are relevant to this code:
|
| @@ -2724,6 +2724,8 @@ def append( | |||
| if copy and is_parameter: | |||
There was a problem hiding this comment.
Above this line (beyond the scope of your changes), I see that we assign to param. My eyes might be playing tricks on me, but is there a point to that?
There was a problem hiding this comment.
I don't know why we bother with the assignment in the lines above - the commit that introduced the assignment (#10977) doesn't say why - before, we didn't have the assignment -, and we drop the variable immediately after. I suspect _validate_expr returns the input just for chaining convenience - there's at least one place that does node = _validate_expr(expr.lift(node)), so that's probably the base.
Either that, or some earlier version of the code (as in, during local development) had more of a call for it.
Mostly this just needed to re-use the existing handling from within `Delay`, and to teach the control-flow builders to look in another special place (`BoxOp.duration`) for potential `Expr` instances to close over.
Summary
Mostly this just needed to re-use the existing handling from within
Delay, and to teach the control-flow builders to look in another special place (BoxOp.duration) for potentialExprinstances to close over.Old PR message
This is currently built on top of a roll-up merge of Kevin's `stretchy-delay` branch, and just unifies the handling between `Delay` and `Box` so that they both support the `Expr` stuff. There's no need for Rust-space handling for `box`, because the entirety of the control-flow system isn't handled in Python-space yet.This might need further modification based on Kevin's work on transpiler support.
This commit will be rebased and amended along with the rest of the
stretchpatch series.Details and comments
Built on top of both #13869 and #13853.