Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Infer partition size for FixedSizeList-backed components #9210

Merged
merged 20 commits into from
Mar 11, 2025

Conversation

oxkitsune
Copy link
Member

Related

Closes #9171

What

This lets the user do:

rr.send_columns(
  "random_3d_points",
  indexes=[times],
  columns=rr.Points3D.columns(positions=np.random.random((len(times), 21, 3))),
)

@oxkitsune oxkitsune added 🐍 Python API Python logging API include in changelog 🪵 Log & send APIs Affects the user-facing API for all languages labels Mar 6, 2025
Copy link

github-actions bot commented Mar 6, 2025

Web viewer built successfully. If applicable, you should also test it:

  • I have tested the web viewer
Result Commit Link Manifest
2bb9ccd https://rerun.io/viewer/pr/9210 +nightly +main

Note: This comment is updated whenever you push a commit.

@oxkitsune oxkitsune force-pushed the gijs/infer-fixed-list-partition branch from edab458 to 91b7f0d Compare March 6, 2025 09:33
Copy link

github-actions bot commented Mar 6, 2025

Latest documentation preview deployed successfully.

Result Commit Link
2bb9ccd https://landing-ahv9zdepe-rerun.vercel.app/docs

Note: This comment is updated whenever you push a commit.

@Wumpf Wumpf self-requested a review March 6, 2025 13:09
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

pixi.lock change is suspicious, why did it change here?
Cool that this just works :O. But needs a test or snippet using this
Could docs/snippets/all/archetypes/points3d_column_updates.py be updated for that?

@Wumpf
Copy link
Member

Wumpf commented Mar 6, 2025

Could docs/snippets/all/archetypes/points3d_column_updates.py be updated for that?

ah the answer is certainly no since inhomogenous partitions sizes can't be detected

@oxkitsune
Copy link
Member Author

pixi.lock change is suspicious, why did it change here?

I think this happened because @jleibs updated on a machine with cuda available, and I synced it on my mac?

@Wumpf
Copy link
Member

Wumpf commented Mar 6, 2025

Or maybe you're running an old pixi. We just updated #9203

@oxkitsune
Copy link
Member Author

Or maybe you're running an old pixi. We just updated #9203

❯ pixi --version
pixi 0.41.4

I think I'm up to date right?

@oxkitsune oxkitsune force-pushed the gijs/infer-fixed-list-partition branch 3 times, most recently from 4c57a92 to 17a250d Compare March 7, 2025 10:27
@oxkitsune oxkitsune requested a review from Wumpf March 7, 2025 14:13
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

love the new test, should have had this from the very start when this utility was first introduced

@oxkitsune oxkitsune force-pushed the gijs/infer-fixed-list-partition branch from 32cac73 to edaaa50 Compare March 10, 2025 11:02
@oxkitsune oxkitsune requested a review from Wumpf March 10, 2025 11:15
@oxkitsune oxkitsune force-pushed the gijs/infer-fixed-list-partition branch from b59201e to 74acbc8 Compare March 10, 2025 14:32
@oxkitsune oxkitsune force-pushed the gijs/infer-fixed-list-partition branch from d98bc0b to 74c9b4b Compare March 11, 2025 09:32
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

phew, lotsa snippet work. looking great

@Wumpf Wumpf self-assigned this Mar 11, 2025
@Wumpf Wumpf merged commit 4ad3e43 into main Mar 11, 2025
@Wumpf Wumpf deleted the gijs/infer-fixed-list-partition branch March 11, 2025 13:34
jprochazk pushed a commit that referenced this pull request Mar 11, 2025
### Related

Closes #9171

### What

This lets the user do:
```py
rr.send_columns(
  "random_3d_points",
  indexes=[times],
  columns=rr.Points3D.columns(positions=np.random.random((len(times), 21, 3))),
)
```

---------

Co-authored-by: Andreas Reich <r_andreas2@web.de>
abey79 pushed a commit that referenced this pull request Mar 12, 2025
…ys (#9262)

### Related

* Follow-up to #9210

### What

In other words, the following snippet broke (which got caught by now-red
main ci! phew!)

```py
rr.send_columns(
    "box",
    indexes=[rr.IndexColumn("tick", sequence=range(1, 101))],
    columns=rr.Transform3D.columns(
        translation=[[0, 0, t / 10.0] for t in range(100)],
        rotation_axis_angle=[
            rr.RotationAxisAngle(axis=[0.0, 1.0, 0.0], radians=truncated_radians(t * 4)) for t in range(100)
        ],
    ),
)
```

`rotation_axis_angle` worked fine, but `translation` broke because the
`columns` call incorrectly assumed that it's 100 columns (correct) each
with a batch size of 3 elements (very wrong).

* [x] pass full-ci
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
include in changelog 🪵 Log & send APIs Affects the user-facing API for all languages 🐍 Python API Python logging API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Infer partition size for FixedSizeList-backed components
2 participants