-
Notifications
You must be signed in to change notification settings - Fork 366
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
Convert the SpaceViewArchetype to use QueryExpression as a batch #5516
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jleibs
added
🟦 blueprint
The data that defines our UI
🐍 Python API
Python logging API
exclude from changelog
PRs with this won't show up in CHANGELOG.md
labels
Mar 15, 2024
5 tasks
emilk
approved these changes
Mar 15, 2024
/// The last rule matching `/world` is `- /world`, so it is excluded. | ||
/// The last rule matching `/world/house` is `+ /world/**`, so it is included. | ||
/// | ||
/// Unstable. Used for the ongoing blueprint experimentations. |
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.
When do we remove the unstable warning?
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.
Good question -- I'll do them all in a blueprint doc cleanup pass.
#5527
jleibs
added a commit
that referenced
this pull request
Mar 15, 2024
…5517) ### What - Initial implementation of: #5288 - Builds on top of: #5516 This is a very dumb first stab at doing variable substitution. - Rather than parse the string to extract potential `$vars` it uses the input environment and blindly tries to substitute all the vars it knows about (currently only `origin`). - The biggest downside of this is we get no feedback when a variable fails to substitute. - There's just enough future complexity handling edge-cases (e.g. mismatched `{`, variable-termination, nested substitutions, etc.) that it might be worth pulling in a proper utility library, though I don't know if there's an obvious rust ecosystem choice here. Working through this uncovered some complexities regarding what we store in different parts of the blueprint. For example, if we do the full substitution immediately when construction the EntityPathFilter, then we can't use that Filter to re-create the query with the variable substitutions. Additionally, we need to know about these substitutions all the way back when evaluating whether we want to keep RecommendedSpaceViews because we need the substitutions applied to do the overlap-checking. I suspect the direction we might want to go in is to split EntityPathFilter into a non-substituted representation, from which we can create a version that executes the substitutions, but I'm not yet sure what the storage should look like. For example, do we just create full `EntityPath`s that contain EntityPathParts with "$origin" in them and then run the substitution on the EntityPath? ### 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 newly built examples: [app.rerun.io](https://app.rerun.io/pr/5517/index.html) * Using examples from latest `main` build: [app.rerun.io](https://app.rerun.io/pr/5517/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [app.rerun.io](https://app.rerun.io/pr/5517/index.html?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/5517) - [Docs preview](https://rerun.io/preview/82c517d5786790176b78fdfbff4a5e261a25f7f0/docs) <!--DOCS-PREVIEW--> - [Examples preview](https://rerun.io/preview/82c517d5786790176b78fdfbff4a5e261a25f7f0/examples) <!--EXAMPLES-PREVIEW--> - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
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
exclude from changelog
PRs with this won't show up in CHANGELOG.md
🐍 Python API
Python logging API
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Working with lists of query expressions is generally much more ergonomic than wrangling newline separated strings in the blueprint APIs.
Checklist
main
build: app.rerun.ionightly
build: app.rerun.io