Implement NodeTemplate + small fixes
#4173
Conversation
| start_idx = len(self.data.ir_nodes) | ||
| with deprecate_random_in_strategy("from {}={!r}", k, s) as check: | ||
| obj = check(self.data.draw(s, observe_as=f"generate:{k}")) | ||
| end_idx = self.data.index_ir | ||
| end_idx = len(self.data.ir_nodes) | ||
| kwargs[k] = obj |
There was a problem hiding this comment.
this is because index_ir has diverged a bit from the buffer concept of "index into the choice sequence". Rather than try to sync a separate variable with that concept like we did with .index for buffers, I think the very few usage sites can just use the more verbose len(nodes).
I think the suggestion reads worse, thoughts on disabling this lint rule? |
| if size + self.length_ir > self.max_length_ir: | ||
| if self.length_ir + size > self.max_length_ir: | ||
| debug_report( | ||
| f"overrun because {self.length_ir} + {size} > {self.max_length_ir=}" |
There was a problem hiding this comment.
| f"overrun because {self.length_ir} + {size} > {self.max_length_ir=}" | |
| f"overrun because {self.length_ir=} + {size=} > {self.max_length_ir=}" |
?
There was a problem hiding this comment.
was semi intentional, but yeah I think with = is fine.
| # we don't consider shrink_towards for unbounded integers. | ||
| # the trivial value should probably be 1 here, not 0. |
There was a problem hiding this comment.
I'm pretty sure we should change this, though maybe in a follow-up PR.
There was a problem hiding this comment.
agree - I was hoping to hold off until ordering is on the ir though, where this will be simpler to implement. If nobody has noticed yet then it should be ok for another month?
Split from #4172. Recommend reviewing by commit.