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

arrow2-convert migration 2: non-serde blueprint types #3855

Merged
merged 9 commits into from
Oct 16, 2023

Conversation

teh-cmc
Copy link
Member

@teh-cmc teh-cmc commented Oct 13, 2023

This PR introduces ObjectKind::Blueprint into our codegen framework.

Blueprint objects are as expressive as Datatype objects (i.e. they don't have the artificial limitations that we impose on Component objects), but still implement the Component trait.
Blueprint objects live in a separate module so as not to pollute LSP/docs/etc.

This allows us to replace existing arrow2_convert-based blueprint types with fbs definitions instead.

This PR does not cover SerdeField shenanigans, that comes next.


arrow2-convert migration PR series:


What

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 demo.rerun.io (if applicable)
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG

@teh-cmc teh-cmc added 🏹 arrow concerning arrow 🚜 refactor Change the code, not the functionality 🟦 blueprint The data that defines our UI exclude from changelog PRs with this won't show up in CHANGELOG.md labels Oct 13, 2023
@teh-cmc teh-cmc changed the base branch from main to cmc/arrow2convert_be_gone October 13, 2023 14:05
@teh-cmc teh-cmc changed the title Cmc/arrow2convert be gone 2 arrow2-convert migration 2: non-serde blueprint types Oct 13, 2023
@teh-cmc teh-cmc changed the title arrow2-convert migration 2: non-serde blueprint types arrow2-convert migration 2: non-serde blueprint types Oct 13, 2023
@teh-cmc teh-cmc force-pushed the cmc/arrow2convert_be_gone_2 branch from 5172b1a to d5b5f3d Compare October 13, 2023 14:08
@teh-cmc teh-cmc marked this pull request as ready for review October 13, 2023 14:46
@emilk emilk self-requested a review October 16, 2023 10:11
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.

Great!

crates/re_types_builder/src/codegen/python.rs Outdated Show resolved Hide resolved
Base automatically changed from cmc/arrow2convert_be_gone to main October 16, 2023 13:02
@teh-cmc teh-cmc force-pushed the cmc/arrow2convert_be_gone_2 branch from 6cc01d7 to e136da9 Compare October 16, 2023 13:08
@teh-cmc teh-cmc merged commit 6d8c4a9 into main Oct 16, 2023
30 checks passed
@teh-cmc teh-cmc deleted the cmc/arrow2convert_be_gone_2 branch October 16, 2023 13:25
teh-cmc added a commit that referenced this pull request Oct 18, 2023
… out dependencies (#3897)

This introduces the `rust.attr.override_crate` attribute, which is
necessary for the serde-codegen story.

This attribute also allows us to generate some fundamental types such as
`InstanceKey` directly into `re_types_core` rather than `re_types`,
making it possible to reduce the number of crates that depend on the
giant `re_types`.

- The `AutoSpaceViews` & `PanelView` components are now back into their
respective home (`re_viewport` & `re_viewer`, respectively).
They were temporarily moved it because we had no support for
`custom_crate`.
They will be joined by their more complicated siblings in the next PR,
which implements the necessary serde-codegen support.

- `InstanceKey`, `Clear` and `ClearIsRecursive` are now generated into
`re_types_core`.
This allows `re_query`, `re_arrow_store` and `re_data_store` to drop
their dependency on `re_types`.

- Generated code now picks up arrow from `re_types_core::external`
instead of importing it directly.
This means crates hosting generated code are not forced into import
`arrow2` directly.

---


`arrow2-convert` migration PR series:
- #3853 
- #3855
- #3897
- #3902
teh-cmc added a commit that referenced this pull request Oct 19, 2023
…3902)

This introduces the `rust.attr.serde_type` attribute, allowing you to
use any `serde`-compatible Rust types in our IDL.
```
table ViewportLayout (
  "attr.rust.derive": "PartialEq",
  "attr.rust.override_crate": "re_viewport"
) {
  space_view_keys: [ubyte] (order: 100, "attr.rust.serde_type": "std::collections::BTreeSet<re_viewer_context::SpaceViewId>");

  tree: [ubyte] (order: 101, "attr.rust.serde_type": "egui_tiles::Tree<re_viewer_context::SpaceViewId>");

  auto_layout: bool (order: 102);
}
```

This unblocks further blueprint experimentations, and is the last
blocker to sunset `arrow2-convert`.

- `SpaceViewComponent`, `SpaceViewMaximized` & `ViewportLayout` are now
all implemented that way.
- `re_viewport` is now free of `arrow2-convert`.

---

`arrow2-convert` migration PR series:
- #3853 
- #3855
- #3897
- #3902
- #3917
teh-cmc added a commit that referenced this pull request Oct 19, 2023
The end of our wonderful journey.

- `NumInstances` control column now has an actual dedicated component
type.
- `EntityPath` is now a component.
- `Into<Cow<Self>>` impls have been cleaned up to generate way less
code.
- `arrow2_convert` is fully gone.


---

`arrow2-convert` migration PR series:
- #3853 
- #3855
- #3897
- #3902
- #3917
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏹 arrow concerning arrow 🟦 blueprint The data that defines our UI exclude from changelog PRs with this won't show up in CHANGELOG.md 🚜 refactor Change the code, not the functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants