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.

Sign up for GitHub

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

Jump to bottom

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

Closed
abey79 opened this issue Sep 19, 2024 · 1 comment · Fixed by #7572
Closed

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

abey79 opened this issue Sep 19, 2024 · 1 comment · Fixed by #7572
Labels
📖 documentation Improvements or additions to documentation enhancement New feature or request ⛃ re_datastore affects the datastore itself
Milestone
0.19 - Dataframe ...

Comments

@abey79
Copy link
Member

abey79 commented Sep 19, 2024 •
edited
Loading

LatestAtQueryHandle::get()'s docstring currently states:

Returns a single [RecordBatch] containing a single row, where each cell corresponds to the latest known value at that particular point in time for each respective ColumnDescriptor.

This is in practice incorrect, and there are at least two situations where no rows are currently returned:

  • if the store is empty, as per the empty_yields_empty() test
  • if no component columns are selected (only control and time column)

It think the later case might be a bug. Regardless, it's probably worth formally specifying the case(s) in which a latest-at query might return nothing.

Also, LatestAtQueryHandle should implement row_count() just like RangeQueryHandle.
@abey79 abey79 added 📖 documentation Improvements or additions to documentation ⛃ re_datastore affects the datastore itself labels Sep 19, 2024
@abey79 abey79 added this to the 0.19 - Dataframe and web video milestone Sep 19, 2024
@abey79 abey79 changed the title Specify in which cases may lead LatestAtQueryHandle::get() to return 0 instead 1 row Specify in which cases may lead LatestAtQueryHandle::get() to return zero instead of one row Sep 19, 2024
@abey79 abey79 changed the title Specify in which cases may lead LatestAtQueryHandle::get() to return zero instead of one row Specify which circumstances lead LatestAtQueryHandle::get() to return zero instead of one row Sep 19, 2024
@abey79 abey79 added the enhancement New feature or request label Sep 19, 2024
@abey79
Copy link
Member Author

abey79 commented Sep 20, 2024

#7453 includes a somewhat ugly workaround to account for unexpected 0-length latest-at results.

@abey79 abey79 mentioned this issue Sep 20, 2024
Merged
7 tasks
abey79 added a commit that referenced this issue Sep 23, 2024
@abey79
Dataframe column visibility can now be controlled (#7453)
f287587 
### What

* Closes #7067
* Builds on #7438 

Add the possibility to control the visibility of column:
- Time columns (except the one corresponding to the query) and component
columns can be hidden by clicking on the on-hover button in their
header.
- A new button in the view title bar with a popup to show/hide any
column.

This also fixes a red warning showing up when latest-at queries return 0
rows (see #7449 for background).

Note to reviewer: the meat is in `space_view_class.rs`, which is ofc
hidden by default by GitHub.


https://github.com/user-attachments/assets/d2b55c63-53db-46dd-9339-b10af98a073b


### TODO

- [x] Fix bad sizing of the popup menu

### 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/7453?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/7453?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/7453)
- [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 mentioned this issue Oct 2, 2024
Merged
6 tasks
abey79 added a commit that referenced this issue Oct 3, 2024
@abey79
Dataframe view update and blueprint API (part 5): switch to `re_dataf…
5d0c7ff 
…rame2` (#7572)

### What

- Fixes #7449

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.

<img width="1704" alt="image"
src="https://github.com/user-attachments/assets/7c57520b-bba8-4f32-8bb9-3a2e51feffe5">


<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/7572?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/7572?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/7572)
- [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
Assignees
No one assigned
Labels
📖 documentation Improvements or additions to documentation enhancement New feature or request ⛃ re_datastore affects the datastore itself
Projects
None yet
Milestone
0.19 - Dataframe and video
Development

Successfully merging a pull request may close this issue.

Dataframe view update and blueprint API (part 5): switch to re_dataframe2 rerun-io/rerun
1 participant
@abey79