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

Make visual time range on views a view property that can be set from python code #6164

Merged
merged 15 commits into from Apr 30, 2024

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Apr 29, 2024

What

And some related refactors that I got engulfed with on this quest:

  • document fate of remaining EntityProperties
  • simplify DataResult formation by dropping overrides from the OverrideContext
    • took the opportunity to call out EntityProperties as legacy_properties in a lot of places which makes it easier to read these changes
  • make it explicit when we want to use latest-at query by introducing QueryRange type

Python API not great yet, will improve and provide examples in an upcoming iteration.

Planned direct follow-ups to this PR:

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!

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

@Wumpf Wumpf added 🐍 Python API Python logging API 🟦 blueprint The data that defines our UI include in changelog labels Apr 29, 2024
Copy link

github-actions bot commented Apr 29, 2024

Deployed docs

Commit Link
39ddac6 https://landing-4rvxl5wmy-rerun.vercel.app/docs

@emilk emilk self-requested a review April 30, 2024 07:12
Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Awesome!

Comment on lines 95 to 96
from: time_range_boundary_to_visible_history_boundary(&time_range.start),
to: time_range_boundary_to_visible_history_boundary(&time_range.end),
Copy link
Member

Choose a reason for hiding this comment

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

Not new in this PR, but this would read much better (and more idiomatic) as

Suggested change
from: time_range_boundary_to_visible_history_boundary(&time_range.start),
to: time_range_boundary_to_visible_history_boundary(&time_range.end),
from: VisibleHistoryBoundary::from(&time_range.start),
to: VisibleHistoryBoundary::from(&time_range.end),

Copy link
Member Author

Choose a reason for hiding this comment

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

but I really want to kill these conversions, not make them nicer 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

did the from'ification on the function name but didn't implement the trait now because I don't want the conversion disease to spread (would need to go to a different module)

crates/re_space_view_time_series/src/util.rs Outdated Show resolved Hide resolved
crates/re_viewer/src/ui/visual_time_range.rs Outdated Show resolved Hide resolved
crates/re_viewer/src/ui/visual_time_range.rs Outdated Show resolved Hide resolved
@Wumpf Wumpf merged commit b7a72cb into main Apr 30, 2024
39 checks passed
@Wumpf Wumpf deleted the andreas/visual-time-range-property branch April 30, 2024 10:09
jleibs pushed a commit that referenced this pull request May 2, 2024
…ntityProperties`) (#6190)

### What

* Fixes #6175

This regressed in #6164. Decided not to roll back things (which would
have been hard) but instead reimplement how these properties are passed
through and make their role more obvious.


### 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/6190?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/6190?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)!

- [PR Build Summary](https://build.rerun.io/pr/6190)
- [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`.

---------

Co-authored-by: Clement Rey <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🟦 blueprint The data that defines our UI include in changelog 🐍 Python API Python logging API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants