Query editor for the dataframe view #7071

abey79 · 2024-08-06T08:19:43Z

What

Fixes Dataframe view: query view property editor #7065
Supersedes WIP - fully-scoped query UI for the dataframe view #6961

This PR introduce a new blueprint archetype and related bespoke UI to configure the query underlying a dataframe view. As a consequence of this new scheme, Visible Time Range is no longer shown in dataframe view selection panel.

Checklist

I have read and agree to Contributor Guide and the Code of Conduct
I've included a screenshot or gif (if applicable)
I have tested the web demo (if applicable):
- Using examples from latest main build: rerun.io/viewer
- Using full set of examples from nightly build: rerun.io/viewer
The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
If applicable, add a new check to the release checklist!
If have noted any breaking changes to the log API in CHANGELOG.md and the migration guide

To run all checks from main, comment on the PR with @rerun-bot full-check.

The `DataframeQuery` archetype does _not_ include selected components nor point-of-view components for the time being. Lots of TODO/WIP

abey79 · 2024-08-07T14:53:45Z

crates/store/re_types/src/blueprint/datatypes/latest_at_query_ext.rs

+impl Default for LatestAtQuery {
+    fn default() -> Self {
+        Self {
+            timeline: Utf8::from("log_time"),
+            time: TimeInt::MAX,
+        }
+    }
+}


This default is neither useful, nor very robust (hardcoded timeline name), but required for reflection to work.

crates/store/re_types/definitions/rerun/blueprint/components/timeline.fbs

abey79 · 2024-08-07T14:55:04Z

crates/store/re_types/src/blueprint/datatypes/time_range_query_ext.rs

+impl Default for TimeRangeQuery {
+    fn default() -> Self {
+        Self {
+            timeline: Utf8::from("log_time"),
+            start: TimeInt::MIN,
+            end: TimeInt::MAX,
+        }
+    }
+}


Again, unfortunate Default to be having.

abey79 · 2024-08-07T14:57:59Z

crates/viewer/re_space_view_dataframe/src/space_view_class.rs

+        else {
+            re_log::warn_once!("Could not find timeline {:?}.", timeline_name.as_str());
+            //TODO(ab): we should have an error for that
+            return Ok(());
+        };


The SpaceViewSystemExecutionError return type is a bit rigid for the various error situation one might encounter.

We can add a SpaceViewSystemExecutionError::Custom maybe?

crates/viewer/re_space_view_dataframe/src/view_query.rs

abey79 · 2024-08-07T15:14:20Z

crates/viewer/re_space_view_dataframe/src/view_query.rs

+        // The presence (or not) of the timeline component determines if the view should follow the
+        // time panel timeline/latest at query, or override it.


How bad is that going to bite me when doing the blueprint api?

abey79 · 2024-08-07T15:36:12Z

rerun_py/rerun_sdk/rerun/blueprint/datatypes/latest_at_query_ext.py

+from __future__ import annotations
+
+from ... import datatypes
+
+
+class LatestAtQueryExt:
+    """Extension for [LatestAtQuery][rerun.blueprint.datatypes.LatestAtQuery]."""
+
+    # This override is required because otherwise the codegen uses `TimeInt(x)`, which is not valid with the custom
+    # `TimeInt.__init__` override.
+
+    @staticmethod
+    def time__field_converter_override(x: datatypes.TimeIntLike) -> datatypes.TimeInt:
+        if isinstance(x, datatypes.TimeInt):
+            return x
+        else:
+            return datatypes.TimeInt(seq=x)


Thanks a ton to mypy because I wasn't gonna catch this one by myself 😅

…and-ui

…and-ui # Conflicts: # crates/store/re_types/definitions/rerun/blueprint/components/dataframe_view_mode.fbs # rerun_cpp/src/rerun/blueprint/archetypes/dataframe_view_mode.cpp # rerun_cpp/src/rerun/blueprint/archetypes/dataframe_view_mode.hpp

crates/store/re_types/definitions/rerun/blueprint/archetypes/dataframe_query.fbs

crates/store/re_types/definitions/rerun/blueprint/components/timeline.fbs

crates/store/re_types/definitions/rerun/blueprint/datatypes/time_range_query.fbs

crates/store/re_types/definitions/rerun/blueprint/components/timeline.fbs

crates/store/re_types/src/blueprint/components/timeline_ext.rs

emilk · 2024-08-09T07:43:34Z

crates/viewer/re_space_view_dataframe/src/space_view_class.rs

+        else {
+            re_log::warn_once!("Could not find timeline {:?}.", timeline_name.as_str());
+            //TODO(ab): we should have an error for that
+            return Ok(());
+        };


We can add a SpaceViewSystemExecutionError::Custom maybe?

gavrelina · 2024-08-09T08:07:53Z

Have a few comments about this, however they may not belong to this particular issue. I leave that up to you!

When segment select is 'Follow timeline' we shall also display what is the value of following that timeline, like what is the timeline and what is the latest at. This educates users on how this control works and creates a connection with through the familiar pattern. Also nicer because if users want to see what is the time value they don't need to gaze all the way down to the timeline. See the little video:

follow.override.mov

some spacing in the list. so the inline spacing subconsciously gives us signals what belongs to what, right? (like whatever is closer to each other = related and vice versa). So see how in the screenshot the space C is much bigger than B or A. This gives an idea that selector is related only to 'view property' and a bit to the 'timeline', but 'showing' is kinda it's own thing. while actually 'showing' and 'timeline' are an entity that should be closer and then they are related to the segment, which is also related to the 'view property' with other things in the panel.

My guess this could be a part of this issue: #6708

…and-ui

abey79 · 2024-08-09T15:18:00Z

@gavrelina

For (1), I tried and it somehow gets super ugly with the order/sort UI next to it. I will postpone to when we deal with that (we'll have to discuss the exact UI at that point anyways):

I've made some adjustment for (2).

Conflict between #7071 and #7128

### What Main broken by #7128, which missed an updated to code recently introduced (#7071) ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested the web demo (if applicable): * Using examples from latest `main` build: [rerun.io/viewer](https://rerun.io/viewer/pr/7139?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [rerun.io/viewer](https://rerun.io/viewer/pr/7139?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG * [x] If applicable, add a new check to the [release checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)! * [x] If have noted any breaking changes to the log API in `CHANGELOG.md` and the migration guide - [PR Build Summary](https://build.rerun.io/pr/7139) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) To run all checks from `main`, comment on the PR with `@rerun-bot full-check`.

abey79 force-pushed the antoine/dataframe-query-property-and-ui branch 2 times, most recently from 2ae9325 to 17e321c Compare August 7, 2024 12:30

abey79 added 6 commits August 7, 2024 14:31

Basic query view property archetype + (very) WIP UI code

9a5f3e5

The `DataframeQuery` archetype does _not_ include selected components nor point-of-view components for the time being. Lots of TODO/WIP

Cleaned-up fbs

3d6024e

Cleaned some todos

9192638

New UI and lots of clean-up

46c1886

Post rebase fixes

e50794c

Hide visible time range for dataframe view

c9fd2d3

abey79 force-pushed the antoine/dataframe-query-property-and-ui branch from 17e321c to c9fd2d3 Compare August 7, 2024 13:12

Cleaned comments

19ced70

abey79 mentioned this pull request Aug 7, 2024

Refactor view resolver such it it enables passing a ViewerContext to SpaceViewClass::default_query_range() #6918

Open

Self review & lint

7d2b88b