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 5: sunset arrow2-convert #3917

Merged
merged 13 commits into from
Oct 19, 2023

Conversation

teh-cmc
Copy link
Member

@teh-cmc teh-cmc commented Oct 18, 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:


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 do-not-merge Do not merge this PR codegen/idl labels Oct 18, 2023
@teh-cmc teh-cmc force-pushed the cmc/arrow2convert_be_gone_4 branch from 0b54344 to ad9c851 Compare October 18, 2023 13:48
@teh-cmc teh-cmc force-pushed the cmc/arrow2convert_be_gone_5 branch from 68e707b to 3d6bae9 Compare October 18, 2023 13:52
crates/re_arrow_store/src/store_gc.rs Outdated Show resolved Hide resolved
crates/re_log_types/src/num_instances.rs Outdated Show resolved Hide resolved
crates/re_types/definitions/rerun/datatypes/uint32.fbs Outdated Show resolved Hide resolved
@@ -574,7 +574,7 @@ fn quote_arrow_field_serializer(
})
.transpose()
})
.collect::<::re_types_core::SerializationResult<Vec<_>>>()?
Copy link
Member

Choose a reason for hiding this comment

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

a ; after a collect that has no let binding? what is going on here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah the whole "who's responsible for the trailing ';'" is a mess in general.

I'll clean that up during the switch to arrow1, likely by introducing a new rule that if your quote block does not do the assignment, then it should not include the trailing comma.

rerun_cpp/src/rerun/datatypes/u_int32.cpp Outdated Show resolved Hide resolved
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.

good riddance!

Base automatically changed from cmc/arrow2convert_be_gone_4 to main October 19, 2023 07:39
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 teh-cmc removed the do-not-merge Do not merge this PR label Oct 19, 2023
@teh-cmc teh-cmc force-pushed the cmc/arrow2convert_be_gone_5 branch from 3d6bae9 to d139fa5 Compare October 19, 2023 08:04
@teh-cmc teh-cmc added the exclude from changelog PRs with this won't show up in CHANGELOG.md label Oct 19, 2023
@teh-cmc teh-cmc merged commit 26654cd into main Oct 19, 2023
24 of 27 checks passed
@teh-cmc teh-cmc deleted the cmc/arrow2convert_be_gone_5 branch October 19, 2023 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏹 arrow concerning arrow codegen/idl exclude from changelog PRs with this won't show up in CHANGELOG.md
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants