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.

Sign up for GitHub

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

Jump to bottom

Redefine PointND components as SoA #2894

Merged
merged 13 commits into from
Aug 2, 2023
Merged

Redefine PointND components as SoA #2894

merged 13 commits into from
Aug 2, 2023

Conversation

teh-cmc
Copy link
Member

@teh-cmc teh-cmc commented Aug 1, 2023 •
edited by github-actions bot
Loading 

diff --git a/crates/re_types/definitions/rerun/components/point2d.fbs b/crates/re_types/definitions/rerun/components/point2d.fbs
index 66d4eb00b..2087a26c6 100644
--- a/crates/re_types/definitions/rerun/components/point2d.fbs
+++ b/crates/re_types/definitions/rerun/components/point2d.fbs
@@ -17,5 +17,5 @@ struct Point2D (
   "attr.rust.derive": "Default, Copy, PartialEq, PartialOrd",
   order: 100
 ) {
-  xy: rerun.datatypes.Point2D (order: 100);
+  xy: rerun.datatypes.Vec2D (order: 100);
 }
diff --git a/crates/re_types/definitions/rerun/components/point3d.fbs b/crates/re_types/definitions/rerun/components/point3d.fbs
index 1785c85ae..838f5bf6b 100644
--- a/crates/re_types/definitions/rerun/components/point3d.fbs
+++ b/crates/re_types/definitions/rerun/components/point3d.fbs
@@ -17,5 +17,5 @@ struct Point3D (
   "attr.rust.derive": "Default, Copy, PartialEq, PartialOrd",
   order: 100
 ) {
-  xy: rerun.datatypes.Point3D (order: 100);
+  xy: rerun.datatypes.Vec3D (order: 100);
 }

Part of #2884

What

Checklist

@teh-cmc teh-cmc added 🐍 Python API Python logging API 🏹 arrow concerning arrow 🦀 Rust API Rust logging API codegen/idl 🌊 C++ API C/C++ API specific labels Aug 1, 2023
@teh-cmc teh-cmc changed the title Cmc/next gen points Redefine PointND components as SoA Aug 1, 2023
@teh-cmc teh-cmc force-pushed the cmc/next_gen_points branch 3 times, most recently from b1d5851 to 8d95752 Compare August 1, 2023 15:18
@Wumpf Wumpf self-requested a review August 1, 2023 15:43
Wumpf
Wumpf approved these changes Aug 2, 2023
View reviewed changes
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

that's it, not more? 😄

@teh-cmc teh-cmc force-pushed the cmc/next_gen_points branch 2 times, most recently from fea2254 to 33a1d26 Compare August 2, 2023 09:44
@teh-cmc
Copy link
Member Author

teh-cmc commented Aug 2, 2023

I just realized we can clean that up even further, we don't even need PointND datatypes anymore (which were weird to begin with).

@teh-cmc teh-cmc mentioned this pull request Aug 2, 2023
Merged
3 tasks
@teh-cmc teh-cmc force-pushed the cmc/next_gen_points branch 2 times, most recently from 692d228 to cdeb0c4 Compare August 2, 2023 15:00
@teh-cmc teh-cmc changed the base branch from main to cmc/fix_vecnd_vs_numpy August 2, 2023 15:01
Base automatically changed from cmc/fix_vecnd_vs_numpy to main August 2, 2023 15:04
teh-cmc added a commit that referenced this pull request Aug 2, 2023
@teh-cmc
Handle older numpy versions / py 3.8 in VecND extensions (#2896)
98701e0 
What the title says.

This is what prevented us from doing the logical thing in `Arrow3D`
extensions (and became even more of an issue after I made the `PointND`
components use `VecND` internally in #2894).

### What

### 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 [demo.rerun.io](https://demo.rerun.io/pr/2896) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2896)
- [Docs
preview](https://rerun.io/preview/pr%3Acmc%2Ffix_vecnd_vs_numpy/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Acmc%2Ffix_vecnd_vs_numpy/examples)
Wumpf
Wumpf approved these changes Aug 2, 2023
View reviewed changes
crates/re_query/src/dataframe_util.rs Outdated
@@ -213,55 +213,33 @@ impl<A: Archetype> ArchetypeView<A> {

#[test]
fn test_df_builder() {
use re_types::components::{Color, Point2D};
use re_types::components::{Color, Radius};

let points = vec![
Copy link
Member

Choose a reason for hiding this comment

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

not points anymore :P

@teh-cmc teh-cmc merged commit 6dcbcc0 into main Aug 2, 2023
16 checks passed
@teh-cmc teh-cmc deleted the cmc/next_gen_points branch August 2, 2023 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Wumpf Wumpf Wumpf approved these changes

Assignees
No one assigned
Labels
🏹 arrow concerning arrow 🌊 C++ API C/C++ API specific codegen/idl 🐍 Python API Python logging API 🦀 Rust API Rust logging API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@teh-cmc @Wumpf