Skip to content

Conversation

antiguru
Copy link
Member

This change removes the IterContainer type and reworks other types and
implementations to function independently. Notably, this is a breaking
change for users of exotic containers:

  • The Inspect trait defers to &C: IntoIterator to reveal items in
    containers. Not all currently used containers provide this. Vec does,
    but Rc/Arc and Column don't and do not plan to in the future. Users
    should pivot to inspect_container instead and internalize the iteration
    in the closure.
  • The DrainContainer implementaiton for Rc/Arc depends on the wrapped
    type implementing IntoIterator for references. This limits where
    wrapped containers that do not provide this can be used. It's mostly
    limited to the core Timely layer that doesn't provide element-by-element
    functionality.

I feel this change is net positive as it defers to the Rust API to iterate
existing types, instead of us providing our own infrastructure. It comes
at a cost of potentially breaking existing code, which seems acceptable.

This change removes the IterContainer type and reworks other types and
implementations to function independently. Notably, this is a breaking
change for users of exotic containers:
* The `Inspect` trait defers to `&C: IntoIterator` to reveal items in
  containers. Not all currently used containers provide this. Vec does,
  but Rc/Arc and Column don't and do not plan to in the future. Users
  should pivot to `inspect_container` instead and internalize the iteration
  in the closure.
* The `DrainContainer` implementaiton for Rc/Arc depends on the wrapped
  type implementing `IntoIterator` for references. This limits where
  wrapped containers that do not provide this can be used. It's mostly
  limited to the core Timely layer that doesn't provide element-by-element
  functionality.

I feel this change is net positive as it defers to the Rust API to iterate
existing types, instead of us providing our own infrastructure. It comes
at a cost of potentially breaking existing code, which seems acceptable.

Signed-off-by: Moritz Hoffmann <[email protected]>
Copy link
Member

@frankmcsherry frankmcsherry left a comment

Choose a reason for hiding this comment

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

I'm good with all of this! Great, even! Moves away from timely as a row-by-row system, and aims at at moving containers of collections around, and if you want the row-by-row interpretation cool but that's some amount of on you.

@frankmcsherry frankmcsherry merged commit a34986d into TimelyDataflow:master Sep 17, 2025
7 checks passed
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.

2 participants