-
Notifications
You must be signed in to change notification settings - Fork 522
Add arrow data tree view and syntax highlighting #10777
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
Conversation
Web viewer built successfully. If applicable, you should also test it:
Note: This comment is updated whenever you push a commit. |
Nice 🤩 |
@lucasmerlin is this ready for review? |
# Conflicts: # crates/viewer/re_component_ui/tests/snapshots/all_components_list_item_narrow/VideoSample_placeholder.png # crates/viewer/re_component_ui/tests/snapshots/all_components_list_item_narrow/custom_small_array_any_value_small_array.png # crates/viewer/re_component_ui/tests/snapshots/all_components_list_item_narrow/one_large_blob_any_value_one_large_blob.png # crates/viewer/re_component_ui/tests/snapshots/all_components_list_item_narrow/one_small_blob_any_value_one_small_blob.png # crates/viewer/re_component_ui/tests/snapshots/all_components_list_item_narrow/two_large_blobs_any_value_two_large_blobs.png # crates/viewer/re_component_ui/tests/snapshots/all_components_list_item_wide/VideoSample_placeholder.png # crates/viewer/re_component_ui/tests/snapshots/all_components_list_item_wide/custom_small_array_any_value_small_array.png # crates/viewer/re_component_ui/tests/snapshots/all_components_list_item_wide/one_large_blob_any_value_one_large_blob.png # crates/viewer/re_component_ui/tests/snapshots/all_components_list_item_wide/one_small_blob_any_value_one_small_blob.png # crates/viewer/re_component_ui/tests/snapshots/all_components_list_item_wide/two_large_blobs_any_value_two_large_blobs.png # crates/viewer/re_ui/src/arrow_ui.rs # crates/viewer/re_ui/tests/snapshots/arrow_ui.png
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make sure we have a test of a BinaryArray
and LargeBinaryArray
in there, for when we get back to #11068
/// E.g. in a ListArray, this is the index in the child list. The index passed to | ||
/// [`ArrowNode::show`] is the index in the parent array. | ||
Index(usize), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this needs some ASCII art and/or a link to the arrow spec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a link to show_index::list_ui
where I've added a more thorough explanation and link to the GenericListArray docs which has an ascii art explanation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh… because it is editable. This is interesting. We should open some sort of issue about this. It is sad that the things that support editing doesn't support syntax highlighting, and the other way around 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reads like a table with a Data type
column with values 0, 1, 2, and a binary
column
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, annoying 🙁 I've added a separate scope so they aren't aligned. It looks visually less appealing now but it's also less confusing. Maybe @gavrelina can have a look at this.
Those are part of the arrow_test_data module 👍🏻 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't do a deep review, but I tried to sufficiently grasp what's going such that I could act on it if I needed to. I added a few comments that would help in the process.
Regardless, the result is stunning!
Related
SyntaxHighlighting
to make it easier for implementor to access theme and tokens #11059What
Instead of formatting arrow data as a String and displaying that, this adds an interactive arrow data explorer:
Screen.Recording.2025-07-30.at.16.19.21.mov
It's still usable if data is rapidly changing:
Screen.Recording.2025-07-30.at.16.21.59.mov
And it even handles large arrays:
Screen.Recording.2025-07-30.at.16.26.02.mov
Here is the example recording I've used to test it with lots of arrow AnyValues: https://rerunio.slack.com/archives/C08TEFXNHB2/p1755162445003819?thread_ts=1755162422.345739&cid=C08TEFXNHB2