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

Dataframe view update and blueprint API (part 5): switch to re_dataframe2 #7572

Merged
merged 5 commits into from
Oct 3, 2024

Conversation

abey79
Copy link
Member

@abey79 abey79 commented Oct 2, 2024

What

This PR does the following:

  • removes the old UI and assorted dead code
  • switches the dependency from re_dataframe to re_dataframe2
  • wires the new UI to the view
  • assorted cleanup

NOTE: latest at and pov do not work yet as they are not supported by re_dataframe2. They will auto-work when it does, as everything is already wired.

image

Part of a series to address #6896 and #7498.

All PRs:

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):
  • 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.

}
let num_sticky_cols = schema
.iter()
.take_while(|cd| matches!(cd, ColumnDescriptor::Control(_) | ColumnDescriptor::Time(_)))
Copy link
Member

Choose a reason for hiding this comment

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

That take_while feels fairly risky for very little gain, why not a filter?

Copy link
Member Author

@abey79 abey79 Oct 3, 2024

Choose a reason for hiding this comment

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

Well we want to have all control columns and time columns to be sticky, but they must appear first in the schema selected columns.

If something weird like this happened, we'd still want to have only 3 sticky columns, not 4 (because that would wrongly include comp1:

row_id  timeA  timeB  comp1  comp2  timeC  comp3

} else {
false
};
//TODO(ab, cmc): we probably need a better way to run a paginated query.
Copy link
Member

Choose a reason for hiding this comment

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

"probably" doing some heavy lifting here 😄

@abey79 abey79 force-pushed the antoine/dfv5-use-dataframe2 branch from 6a3c341 to fb2064e Compare October 3, 2024 08:36
abey79 added a commit that referenced this pull request Oct 3, 2024
…ct and improve doc generation (#7515)

### What

This PR introduces a stub blueprint view object for the dataframe view.
It doesn't contain any actual parameters (query, etc.), but is
sufficient to "spawn" a view. This PR also improves doc generation by
special casing the dataframe view (it accepts all archetypes and
inversely).

(Note: the snippet will be much improved in later PRs.)

<img width="673" alt="image"
src="https://github.com/user-attachments/assets/8a945f46-a6a8-44c6-ab88-db8d19e6992a">


![image](https://github.com/user-attachments/assets/0b1b4db5-5afa-41f9-914d-af04c21e34e0)


<hr>

Part of a series to address #6896 and #7498.

All PRs:
- #7515
- #7516
- #7527 
- #7545
- #7551
- #7572
- #7573

### 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/7515?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/7515?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/7515)
- [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 abey79 force-pushed the antoine/dfv5-use-dataframe2 branch from fb2064e to 7dc024a Compare October 3, 2024 08:39
abey79 added a commit that referenced this pull request Oct 3, 2024
…he new query property (#7516)

### What

This PR introduces a new `DataframeQueryV2` view property archetype
which models the query according to the new dataframe API design
(#7455) and the feature we
actually want to support in the dataframe view
(#7497).

At this point, the new archetype is **NOT** used yet. It just lives
alongside the previous iteration, which is still used by the actual
view. The swap will occur later.

<hr>

Part of a series to address #6896 and #7498.

All PRs:
- #7515
- #7516
- #7527 
- #7545
- #7551
- #7572
- #7573

### 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/7516?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/7516?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/7516)
- [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 abey79 force-pushed the antoine/dfv5-use-dataframe2 branch from 7dc024a to a00c309 Compare October 3, 2024 08:48
abey79 added a commit that referenced this pull request Oct 3, 2024
…7527)

### What

This PR adds a wrapper over the query view property introduced in #7516
and implement the selection panel UI for it. This UI is functional (e.g.
filter event entity and component auto-suggestion, etc.), but it is
still ignored by the view itself and is displayed alongside the original
(still functional) UI.

<img width="424" alt="image"
src="https://github.com/user-attachments/assets/edf10b2a-1705-4f4e-93cc-e8064e902626">

<hr>

Part of a series to address #6896 and #7498.

All PRs:
- #7515
- #7516
- #7527 
- #7545
- #7551
- #7572
- #7573

### 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/7527?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/7527?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/7527)
- [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 abey79 force-pushed the antoine/dfv5-use-dataframe2 branch from a00c309 to d1e4934 Compare October 3, 2024 09:01
@abey79 abey79 force-pushed the antoine/dfv5-use-dataframe2 branch 2 times, most recently from cadc68e to 715244e Compare October 3, 2024 09:23
abey79 added a commit that referenced this pull request Oct 3, 2024
…bject for column visibility (#7545)

### What

This PR wires the column visibility code to the new query object, and
cleans up the old code and related blueprint object. At this point the
dataframe view is in this weird spot where the old ui is used for
everything _but column visibility_.

**Reviewer**: `space_view_class.rs` is where the interesting stuff is
(and is collapsed by default by GH).

<hr>

Part of a series to address #6896 and #7498.

All PRs:
- #7515
- #7516
- #7527 
- #7545
- #7551
- #7572
- #7573

### 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/7545?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/7545?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/7545)
- [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`.
…code
@abey79 abey79 force-pushed the antoine/dfv5-use-dataframe2 branch from 715244e to a833d30 Compare October 3, 2024 09:36
@abey79 abey79 force-pushed the antoine/dfv5-use-dataframe2 branch from a833d30 to f1fa338 Compare October 3, 2024 11:18
abey79 added a commit that referenced this pull request Oct 3, 2024
### What

This PR implements the blueprint API for the dataframe view:

```python
blueprint = rrb.Blueprint(
    rrb.DataframeView(
        origin="/trig",
        query=rrb.archetypes.DataframeQueryV2(
            timeline="t",
            filter_by_range=(rr.TimeInt(seconds=0), rr.TimeInt(seconds=20)),
            filter_by_event="/trig/tan_sparse:Scalar",
            select=["t", "log_tick", "/trig/sin:Scalar", "/trig/cos:Scalar", "/trig/tan_sparse:Scalar"],
        ),
    ),
)
```

The blueprint API is now functional and can be used to configure
everything in the new UI, which is _still_ mostly inoperative (except
for column visibility—this can be tested end-to-end).

<hr>

Part of a series to address #6896 and #7498.

All PRs:
- #7515
- #7516
- #7527 
- #7545
- #7551
- #7572
- #7573

### 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/7551?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/7551?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/7551)
- [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`.
Base automatically changed from antoine/dfv4-blueprint-api to main October 3, 2024 11:31
An error occurred while trying to automatically change base from antoine/dfv4-blueprint-api to main October 3, 2024 11:31
@abey79 abey79 force-pushed the antoine/dfv5-use-dataframe2 branch from f1fa338 to 07a0094 Compare October 3, 2024 11:32
@abey79 abey79 removed the do-not-merge Do not merge this PR label Oct 3, 2024
@abey79 abey79 merged commit 5d0c7ff into main Oct 3, 2024
33 of 35 checks passed
@abey79 abey79 deleted the antoine/dfv5-use-dataframe2 branch October 3, 2024 12:20
abey79 added a commit that referenced this pull request Oct 3, 2024
### What

- Closes #6896 
- Closes #7498

Final PR in the series. Pure cleanup.
- removes dead fbs
- rename stuff
- mark stuff as unreleased
- rename `schema` to `view_columns` or `selected_columns`, depending on
context (see [this
comment](#7527 (comment)))

<hr>

Part of a series to address #6896 and #7498.

All PRs:
- #7515
- #7516
- #7527 
- #7545
- #7551
- #7572
- #7573


### 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/7573?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/7573?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/7573)
- [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`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat-dataframe-view Everything related to the dataframe view ui concerns graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Specify which circumstances lead LatestAtQueryHandle::get() to return zero instead of one row
3 participants