-
Notifications
You must be signed in to change notification settings - Fork 329
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
Use enum
in codegen to specify constant-constants
#3384
Comments
would also be useful for log level strings! |
...and for blueprint container kind. |
There are two ways we can encode the enums: using names ("Up", "Down", …), or using integers (1, 2, …). I think using the names is superior, as it means the arrow data is self-descriptive. I believe the best way to encode these in Arrow is by using a Sparse Union with the NULL type for each field. This means only 8 bits should be required for each value. |
### What * Part of #3384 Small restructure and refactor to improve error reporting in the codegen, especially when it comes to `enum` being unimplemented. See commit messages. ### 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/4786/index.html) * Using examples from latest `main` build: [app.rerun.io](https://app.rerun.io/pr/4786/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/4786/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 - [PR Build Summary](https://build.rerun.io/pr/4786) - [Docs preview](https://rerun.io/preview/3e8e9722b7d64b28ca55453152ab80d259600cfe/docs) <!--DOCS-PREVIEW--> - [Examples preview](https://rerun.io/preview/3e8e9722b7d64b28ca55453152ab80d259600cfe/examples) <!--EXAMPLES-PREVIEW--> - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
#4867) ### What This is the basic pattern we expect to see for all of these view-configuration type tasks: - Add a new archetype / component - Add logic to the TimeSeries space-view-class implementation that both reads and writes the components directly from the blueprint store. This generally seems to be much easier to think about than dealing with the EntityProperties struct as this was handled previously as it's exclusively a contact between the TimeSeries archetype and the TimeSeriesSpaceView implementation. Open question: - Should we suffix all our view archetypes with something like `TimeSeriesView`? Needs #4609 to land first since I developed there before rebasing to main. Will be nice to come back and clean this up again once #3384 is done. ### 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/4867/index.html) * Using examples from latest `main` build: [app.rerun.io](https://app.rerun.io/pr/4867/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/4867/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 - [PR Build Summary](https://build.rerun.io/pr/4867) - [Docs preview](https://rerun.io/preview/d76ed7714f29407bc1d4a62779bfa2894a10fa90/docs) <!--DOCS-PREVIEW--> - [Examples preview](https://rerun.io/preview/d76ed7714f29407bc1d4a62779bfa2894a10fa90/examples) <!--EXAMPLES-PREVIEW--> - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
### What * Part of #3384 This implements C-style `enum` support in the codegen, with a full Rust implementation. There is no example included, because the Python and C++ codegen is still emitting an error when attempting to use `enum`, but I have tested the codegen a bit locally, and the generated code at least _compiles_. Future PRs: * Python codegen * C++ codegen * Tests * Converting existing types to `enum` While working on the PR I also cleaned up some other codegen stuff, but reviewing everything commit-by-commit should make it all 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 newly built examples: [app.rerun.io](https://app.rerun.io/pr/5291/index.html) * Using examples from latest `main` build: [app.rerun.io](https://app.rerun.io/pr/5291/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/5291/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/5291) - [Docs preview](https://rerun.io/preview/017812cb07a396308b070e411334edcf2a4cf352/docs) <!--DOCS-PREVIEW--> - [Examples preview](https://rerun.io/preview/017812cb07a396308b070e411334edcf2a4cf352/examples) <!--EXAMPLES-PREVIEW--> - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
### What * Part of #3384 ### Coming later * Python * Tests * Switching to using `enum` for existing types ### 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/5314/index.html) * Using examples from latest `main` build: [app.rerun.io](https://app.rerun.io/pr/5314/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/5314/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/5314) - [Docs preview](https://rerun.io/preview/cdd09f81a3a8fa16c2dbbffca7403165bc907739/docs) <!--DOCS-PREVIEW--> - [Examples preview](https://rerun.io/preview/cdd09f81a3a8fa16c2dbbffca7403165bc907739/examples) <!--EXAMPLES-PREVIEW--> - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) --------- Co-authored-by: Andreas Reich <andreas@rerun.io>
### What * Part of #3384 Even includes codegen of the arrow serialization, which is a first for Python. Best reviewed commit-by-commit ### 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/5319/index.html) * Using examples from latest `main` build: [app.rerun.io](https://app.rerun.io/pr/5319/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/5319/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/5319) - [Docs preview](https://rerun.io/preview/2f1d162e5f221d9c0a78b1bcff1524fec67e110c/docs) <!--DOCS-PREVIEW--> - [Examples preview](https://rerun.io/preview/2f1d162e5f221d9c0a78b1bcff1524fec67e110c/examples) <!--EXAMPLES-PREVIEW--> - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
### What * Closes #3384 Also replaces `Corner2D` and `ContainerKind`, but those are blueprint only and thus not user facing (yet). The user-facing code should stay mostly the same. I also improved the generated documentation for all components and datatypes ### 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/5336/index.html) * Using examples from latest `main` build: [app.rerun.io](https://app.rerun.io/pr/5336/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/5336/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/5336) - [Docs preview](https://rerun.io/preview/5819de7329032052d5291e1696706667009c8fc7/docs) <!--DOCS-PREVIEW--> - [Examples preview](https://rerun.io/preview/5819de7329032052d5291e1696706667009c8fc7/examples) <!--EXAMPLES-PREVIEW--> - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) --------- Co-authored-by: Clement Rey <cr.rey.clement@gmail.com>
Our
enum
s currently get converted into classes that implement arrow unions, which I don't believe is correct. The more common pattern of using enums in fbs files, for example:Should map to an appropriate enum of constants in each language.
The text was updated successfully, but these errors were encountered: