Skip to content

Commit

Permalink
Don't use builtin required anymore, introduce nullable instead (#…
Browse files Browse the repository at this point in the history
…2619)

Fixes #2525
Closes #2524

---

### 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/2619) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2619)
- [Docs preview](https://rerun.io/preview/pr%3Acmc%2Fattr_nullable/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Acmc%2Fattr_nullable/examples)
  • Loading branch information
teh-cmc authored Jul 7, 2023
1 parent a6568a1 commit 670812d
Show file tree
Hide file tree
Showing 14 changed files with 141 additions and 118 deletions.
8 changes: 8 additions & 0 deletions crates/re_types/definitions/fbs/attributes.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,11 @@ attribute "order";
/// table when defining a union, cannot put arrays (as opposed to vectors) into tables, etc) that
/// do not apply in our case.
attribute "transparent";

/// If specified on a field, the field becomes nullable, which affects both its native as well as
/// its arrow datatypes.
///
/// NOTE: We do not use flatbuffers' builtin `required` attribute because A) it has many
/// limitations that do not make sense for our use case and B) our overall data model is built
/// around the idea of nullability, rather than requirements (i.e. the exact opposite).
attribute "nullable";
16 changes: 8 additions & 8 deletions crates/re_types/definitions/rerun/archetypes/points2d.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -34,34 +34,34 @@ table Points2D (
// --- Required ---

/// All the actual 2D points that make up the point cloud.
points: [rerun.components.Point2D] ("attr.rerun.component_required", required, order: 1000);
points: [rerun.components.Point2D] ("attr.rerun.component_required", order: 1000);

// --- Recommended ---

/// Optional radii for the points, effectively turning them into circles.
radii: [rerun.components.Radius] ("attr.rerun.component_recommended", order: 2000);
radii: [rerun.components.Radius] ("attr.rerun.component_recommended", nullable, order: 2000);

/// Optional colors for the points.
///
/// \python The colors are interpreted as RGB or RGBA in sRGB gamma-space,
/// \python As either 0-1 floats or 0-255 integers, with separate alpha.
colors: [rerun.components.Color] ("attr.rerun.component_recommended", order: 2100);
colors: [rerun.components.Color] ("attr.rerun.component_recommended", nullable, order: 2100);

// --- Optional ---

/// Optional text labels for the points.
labels: [rerun.components.Label] ("attr.rerun.component_optional", order: 3000);
labels: [rerun.components.Label] ("attr.rerun.component_optional", nullable, order: 3000);

/// An optional floating point value that specifies the 2D drawing order.
/// Objects with higher values are drawn on top of those with lower values.
///
/// The default for 2D points is 30.0.
draw_order: rerun.components.DrawOrder ("attr.rerun.component_optional", order: 3100);
draw_order: rerun.components.DrawOrder ("attr.rerun.component_optional", nullable, order: 3100);

/// Optional class Ids for the points.
///
/// The class ID provides colors and labels if not specified explicitly.
class_ids: [rerun.components.ClassId] ("attr.rerun.component_optional", order: 3200);
class_ids: [rerun.components.ClassId] ("attr.rerun.component_optional", nullable, order: 3200);

/// Optional keypoint IDs for the points, identifying them within a class.
///
Expand All @@ -71,8 +71,8 @@ table Points2D (
/// with `class_id`).
/// E.g. the classification might be 'Person' and the keypoints refer to joints on a
/// detected skeleton.
keypoint_ids: [rerun.components.KeypointId] ("attr.rerun.component_optional", order: 3300);
keypoint_ids: [rerun.components.KeypointId] ("attr.rerun.component_optional", nullable, order: 3300);

/// Unique identifiers for each individual point in the batch.
instance_keys: [rerun.components.InstanceKey] ("attr.rerun.component_optional", order: 3400);
instance_keys: [rerun.components.InstanceKey] ("attr.rerun.component_optional", nullable, order: 3400);
}
2 changes: 1 addition & 1 deletion crates/re_types/definitions/rerun/components/label.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@ table Label (
"attr.rust.tuple_struct",
order: 100
) {
value: string (required, order: 100);
value: string (order: 100);
}
104 changes: 52 additions & 52 deletions crates/re_types/definitions/rerun/testing/archetypes/fuzzy.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -15,60 +15,60 @@ table AffixFuzzer1 (
"attr.rust.derive": "PartialEq",
order: 100
) {
fuzz1001: rerun.testing.components.AffixFuzzer1 ("attr.rerun.component_required", required, order: 1001);
fuzz1002: rerun.testing.components.AffixFuzzer2 ("attr.rerun.component_required", required, order: 1002);
fuzz1003: rerun.testing.components.AffixFuzzer3 ("attr.rerun.component_required", required, order: 1003);
fuzz1004: rerun.testing.components.AffixFuzzer4 ("attr.rerun.component_required", required, order: 1004);
fuzz1005: rerun.testing.components.AffixFuzzer5 ("attr.rerun.component_required", required, order: 1005);
fuzz1006: rerun.testing.components.AffixFuzzer6 ("attr.rerun.component_required", required, order: 1006);
fuzz1007: rerun.testing.components.AffixFuzzer7 ("attr.rerun.component_required", required, order: 1007);
fuzz1008: rerun.testing.components.AffixFuzzer8 ("attr.rerun.component_required", required, order: 1008);
fuzz1009: rerun.testing.components.AffixFuzzer9 ("attr.rerun.component_required", required, order: 1009);
fuzz1010: rerun.testing.components.AffixFuzzer10 ("attr.rerun.component_required", required, order: 1010);
fuzz1011: rerun.testing.components.AffixFuzzer11 ("attr.rerun.component_required", required, order: 1011);
fuzz1012: rerun.testing.components.AffixFuzzer12 ("attr.rerun.component_required", required, order: 1012);
fuzz1013: rerun.testing.components.AffixFuzzer13 ("attr.rerun.component_required", required, order: 1013);
fuzz1001: rerun.testing.components.AffixFuzzer1 ("attr.rerun.component_required", order: 1001);
fuzz1002: rerun.testing.components.AffixFuzzer2 ("attr.rerun.component_required", order: 1002);
fuzz1003: rerun.testing.components.AffixFuzzer3 ("attr.rerun.component_required", order: 1003);
fuzz1004: rerun.testing.components.AffixFuzzer4 ("attr.rerun.component_required", order: 1004);
fuzz1005: rerun.testing.components.AffixFuzzer5 ("attr.rerun.component_required", order: 1005);
fuzz1006: rerun.testing.components.AffixFuzzer6 ("attr.rerun.component_required", order: 1006);
fuzz1007: rerun.testing.components.AffixFuzzer7 ("attr.rerun.component_required", order: 1007);
fuzz1008: rerun.testing.components.AffixFuzzer8 ("attr.rerun.component_required", order: 1008);
fuzz1009: rerun.testing.components.AffixFuzzer9 ("attr.rerun.component_required", order: 1009);
fuzz1010: rerun.testing.components.AffixFuzzer10 ("attr.rerun.component_required", order: 1010);
fuzz1011: rerun.testing.components.AffixFuzzer11 ("attr.rerun.component_required", order: 1011);
fuzz1012: rerun.testing.components.AffixFuzzer12 ("attr.rerun.component_required", order: 1012);
fuzz1013: rerun.testing.components.AffixFuzzer13 ("attr.rerun.component_required", order: 1013);

fuzz1101: [rerun.testing.components.AffixFuzzer1] ("attr.rerun.component_required", required, order: 1101);
fuzz1102: [rerun.testing.components.AffixFuzzer2] ("attr.rerun.component_required", required, order: 1102);
fuzz1103: [rerun.testing.components.AffixFuzzer3] ("attr.rerun.component_required", required, order: 1103);
fuzz1104: [rerun.testing.components.AffixFuzzer4] ("attr.rerun.component_required", required, order: 1104);
fuzz1105: [rerun.testing.components.AffixFuzzer5] ("attr.rerun.component_required", required, order: 1105);
fuzz1106: [rerun.testing.components.AffixFuzzer6] ("attr.rerun.component_required", required, order: 1106);
fuzz1107: [rerun.testing.components.AffixFuzzer7] ("attr.rerun.component_required", required, order: 1107);
fuzz1108: [rerun.testing.components.AffixFuzzer8] ("attr.rerun.component_required", required, order: 1108);
fuzz1109: [rerun.testing.components.AffixFuzzer9] ("attr.rerun.component_required", required, order: 1109);
fuzz1110: [rerun.testing.components.AffixFuzzer10] ("attr.rerun.component_required", required, order: 1110);
fuzz1111: [rerun.testing.components.AffixFuzzer11] ("attr.rerun.component_required", required, order: 1111);
fuzz1112: [rerun.testing.components.AffixFuzzer12] ("attr.rerun.component_required", required, order: 1112);
fuzz1113: [rerun.testing.components.AffixFuzzer13] ("attr.rerun.component_required", required, order: 1113);
fuzz1101: [rerun.testing.components.AffixFuzzer1] ("attr.rerun.component_required", order: 1101);
fuzz1102: [rerun.testing.components.AffixFuzzer2] ("attr.rerun.component_required", order: 1102);
fuzz1103: [rerun.testing.components.AffixFuzzer3] ("attr.rerun.component_required", order: 1103);
fuzz1104: [rerun.testing.components.AffixFuzzer4] ("attr.rerun.component_required", order: 1104);
fuzz1105: [rerun.testing.components.AffixFuzzer5] ("attr.rerun.component_required", order: 1105);
fuzz1106: [rerun.testing.components.AffixFuzzer6] ("attr.rerun.component_required", order: 1106);
fuzz1107: [rerun.testing.components.AffixFuzzer7] ("attr.rerun.component_required", order: 1107);
fuzz1108: [rerun.testing.components.AffixFuzzer8] ("attr.rerun.component_required", order: 1108);
fuzz1109: [rerun.testing.components.AffixFuzzer9] ("attr.rerun.component_required", order: 1109);
fuzz1110: [rerun.testing.components.AffixFuzzer10] ("attr.rerun.component_required", order: 1110);
fuzz1111: [rerun.testing.components.AffixFuzzer11] ("attr.rerun.component_required", order: 1111);
fuzz1112: [rerun.testing.components.AffixFuzzer12] ("attr.rerun.component_required", order: 1112);
fuzz1113: [rerun.testing.components.AffixFuzzer13] ("attr.rerun.component_required", order: 1113);

fuzz2001: rerun.testing.components.AffixFuzzer1 ("attr.rerun.component_optional", order: 2001);
fuzz2002: rerun.testing.components.AffixFuzzer2 ("attr.rerun.component_optional", order: 2002);
fuzz2003: rerun.testing.components.AffixFuzzer3 ("attr.rerun.component_optional", order: 2003);
fuzz2004: rerun.testing.components.AffixFuzzer4 ("attr.rerun.component_optional", order: 2004);
fuzz2005: rerun.testing.components.AffixFuzzer5 ("attr.rerun.component_optional", order: 2005);
fuzz2006: rerun.testing.components.AffixFuzzer6 ("attr.rerun.component_optional", order: 2006);
fuzz2007: rerun.testing.components.AffixFuzzer7 ("attr.rerun.component_optional", order: 2007);
fuzz2008: rerun.testing.components.AffixFuzzer8 ("attr.rerun.component_optional", order: 2008);
fuzz2009: rerun.testing.components.AffixFuzzer9 ("attr.rerun.component_optional", order: 2009);
fuzz2010: rerun.testing.components.AffixFuzzer10 ("attr.rerun.component_optional", order: 2010);
fuzz2011: rerun.testing.components.AffixFuzzer11 ("attr.rerun.component_optional", order: 2011);
fuzz2012: rerun.testing.components.AffixFuzzer12 ("attr.rerun.component_optional", order: 2012);
fuzz2013: rerun.testing.components.AffixFuzzer13 ("attr.rerun.component_optional", order: 2013);
fuzz2001: rerun.testing.components.AffixFuzzer1 ("attr.rerun.component_optional", nullable, order: 2001);
fuzz2002: rerun.testing.components.AffixFuzzer2 ("attr.rerun.component_optional", nullable, order: 2002);
fuzz2003: rerun.testing.components.AffixFuzzer3 ("attr.rerun.component_optional", nullable, order: 2003);
fuzz2004: rerun.testing.components.AffixFuzzer4 ("attr.rerun.component_optional", nullable, order: 2004);
fuzz2005: rerun.testing.components.AffixFuzzer5 ("attr.rerun.component_optional", nullable, order: 2005);
fuzz2006: rerun.testing.components.AffixFuzzer6 ("attr.rerun.component_optional", nullable, order: 2006);
fuzz2007: rerun.testing.components.AffixFuzzer7 ("attr.rerun.component_optional", nullable, order: 2007);
fuzz2008: rerun.testing.components.AffixFuzzer8 ("attr.rerun.component_optional", nullable, order: 2008);
fuzz2009: rerun.testing.components.AffixFuzzer9 ("attr.rerun.component_optional", nullable, order: 2009);
fuzz2010: rerun.testing.components.AffixFuzzer10 ("attr.rerun.component_optional", nullable, order: 2010);
fuzz2011: rerun.testing.components.AffixFuzzer11 ("attr.rerun.component_optional", nullable, order: 2011);
fuzz2012: rerun.testing.components.AffixFuzzer12 ("attr.rerun.component_optional", nullable, order: 2012);
fuzz2013: rerun.testing.components.AffixFuzzer13 ("attr.rerun.component_optional", nullable, order: 2013);

fuzz2101: [rerun.testing.components.AffixFuzzer1] ("attr.rerun.component_optional", order: 2101);
fuzz2102: [rerun.testing.components.AffixFuzzer2] ("attr.rerun.component_optional", order: 2102);
fuzz2103: [rerun.testing.components.AffixFuzzer3] ("attr.rerun.component_optional", order: 2103);
fuzz2104: [rerun.testing.components.AffixFuzzer4] ("attr.rerun.component_optional", order: 2104);
fuzz2105: [rerun.testing.components.AffixFuzzer5] ("attr.rerun.component_optional", order: 2105);
fuzz2106: [rerun.testing.components.AffixFuzzer6] ("attr.rerun.component_optional", order: 2106);
fuzz2107: [rerun.testing.components.AffixFuzzer7] ("attr.rerun.component_optional", order: 2107);
fuzz2108: [rerun.testing.components.AffixFuzzer8] ("attr.rerun.component_optional", order: 2108);
fuzz2109: [rerun.testing.components.AffixFuzzer9] ("attr.rerun.component_optional", order: 2109);
fuzz2110: [rerun.testing.components.AffixFuzzer10] ("attr.rerun.component_optional", order: 2110);
fuzz2111: [rerun.testing.components.AffixFuzzer11] ("attr.rerun.component_optional", order: 2111);
fuzz2112: [rerun.testing.components.AffixFuzzer12] ("attr.rerun.component_optional", order: 2112);
fuzz2113: [rerun.testing.components.AffixFuzzer13] ("attr.rerun.component_optional", order: 2113);
fuzz2101: [rerun.testing.components.AffixFuzzer1] ("attr.rerun.component_optional", nullable, order: 2101);
fuzz2102: [rerun.testing.components.AffixFuzzer2] ("attr.rerun.component_optional", nullable, order: 2102);
fuzz2103: [rerun.testing.components.AffixFuzzer3] ("attr.rerun.component_optional", nullable, order: 2103);
fuzz2104: [rerun.testing.components.AffixFuzzer4] ("attr.rerun.component_optional", nullable, order: 2104);
fuzz2105: [rerun.testing.components.AffixFuzzer5] ("attr.rerun.component_optional", nullable, order: 2105);
fuzz2106: [rerun.testing.components.AffixFuzzer6] ("attr.rerun.component_optional", nullable, order: 2106);
fuzz2107: [rerun.testing.components.AffixFuzzer7] ("attr.rerun.component_optional", nullable, order: 2107);
fuzz2108: [rerun.testing.components.AffixFuzzer8] ("attr.rerun.component_optional", nullable, order: 2108);
fuzz2109: [rerun.testing.components.AffixFuzzer9] ("attr.rerun.component_optional", nullable, order: 2109);
fuzz2110: [rerun.testing.components.AffixFuzzer10] ("attr.rerun.component_optional", nullable, order: 2110);
fuzz2111: [rerun.testing.components.AffixFuzzer11] ("attr.rerun.component_optional", nullable, order: 2111);
fuzz2112: [rerun.testing.components.AffixFuzzer12] ("attr.rerun.component_optional", nullable, order: 2112);
fuzz2113: [rerun.testing.components.AffixFuzzer13] ("attr.rerun.component_optional", nullable, order: 2113);
}

26 changes: 13 additions & 13 deletions crates/re_types/definitions/rerun/testing/components/fuzzy.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -12,93 +12,93 @@ table AffixFuzzer1 (
"attr.rust.derive": "PartialEq",
order: 100
) {
single_required: rerun.testing.datatypes.AffixFuzzer1 (required, order: 101);
single_required: rerun.testing.datatypes.AffixFuzzer1 (order: 101);
}

table AffixFuzzer2 (
"attr.rust.derive": "PartialEq",
"attr.rust.tuple_struct",
order: 200
) {
single_required: rerun.testing.datatypes.AffixFuzzer1 (required, order: 102);
single_required: rerun.testing.datatypes.AffixFuzzer1 (order: 102);
}

table AffixFuzzer3 (
"attr.rust.derive": "PartialEq",
order: 300
) {
single_required: rerun.testing.datatypes.AffixFuzzer1 (required, order: 103);
single_required: rerun.testing.datatypes.AffixFuzzer1 (order: 103);
}

table AffixFuzzer4 (
"attr.rust.derive": "PartialEq",
order: 400
) {
single_optional: rerun.testing.datatypes.AffixFuzzer1 (order: 104);
single_optional: rerun.testing.datatypes.AffixFuzzer1 (nullable, order: 104);
}

table AffixFuzzer5 (
"attr.rust.derive": "PartialEq",
"attr.rust.tuple_struct",
order: 500
) {
single_optional: rerun.testing.datatypes.AffixFuzzer1 (order: 105);
single_optional: rerun.testing.datatypes.AffixFuzzer1 (nullable, order: 105);
}

table AffixFuzzer6 (
"attr.rust.derive": "PartialEq",
order: 600
) {
single_optional: rerun.testing.datatypes.AffixFuzzer1 (order: 106);
single_optional: rerun.testing.datatypes.AffixFuzzer1 (nullable, order: 106);
}

table AffixFuzzer7 (
"attr.rust.derive": "PartialEq",
order: 700
) {
many_optional: [rerun.testing.datatypes.AffixFuzzer1] (order: 107);
many_optional: [rerun.testing.datatypes.AffixFuzzer1] (nullable, order: 107);
}

table AffixFuzzer8 (
"attr.rust.derive": "PartialEq",
order: 800
) {
single_float_optional: float (order: 108);
single_float_optional: float (nullable, order: 108);
}

table AffixFuzzer9 (
"attr.rust.derive": "PartialEq, Eq",
order: 900
) {
single_string_required: string (required, order: 109);
single_string_required: string (order: 109);
}

table AffixFuzzer10 (
"attr.rust.derive": "PartialEq, Eq",
order: 1000
) {
single_string_optional: string (order: 110);
single_string_optional: string (nullable, order: 110);
}

table AffixFuzzer11 (
"attr.rust.derive": "PartialEq",
order: 1100
) {
many_floats_optional: [float] (order: 111);
many_floats_optional: [float] (nullable, order: 111);
}

table AffixFuzzer12 (
"attr.rust.derive": "PartialEq, Eq",
order: 1200
) {
many_strings_required: [string] (required, order: 112);
many_strings_required: [string] (order: 112);
}

table AffixFuzzer13 (
"attr.rust.derive": "PartialEq, Eq",
order: 1300
) {
many_strings_optional: [string] (order: 113);
many_strings_optional: [string] (nullable, order: 113);
}

// TODO(cmc): the ugly bug we need to take care of at some point
Expand Down
18 changes: 9 additions & 9 deletions crates/re_types/definitions/rerun/testing/datatypes/fuzzy.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ table AffixFuzzer1 (
"attr.rust.derive": "PartialEq",
order: 100
) {
single_float_optional: float (order: 101);
single_string_required: string (order: 102, required);
single_string_optional: string (order: 103);
many_floats_optional: [float] (order: 104);
many_strings_required: [string] (order: 105, required);
many_strings_optional: [string] (order: 106);
flattened_scalar: VeryDeeplyFlattenedScalar (order: 107, required, transparent);
almost_flattened_scalar: SurprisinglyShallowScalar (order: 108, required, transparent);
single_float_optional: float (nullable, order: 101);
single_string_required: string (order: 102);
single_string_optional: string (nullable, order: 103);
many_floats_optional: [float] (nullable, order: 104);
many_strings_required: [string] (order: 105);
many_strings_optional: [string] (nullable, order: 106);
flattened_scalar: VeryDeeplyFlattenedScalar (order: 107, transparent);
almost_flattened_scalar: SurprisinglyShallowScalar (order: 108, transparent);
}

table AffixFuzzer2 (
Expand All @@ -42,5 +42,5 @@ table AffixFuzzer2 (
"attr.rust.tuple_struct",
order: 200
) {
single_float_optional: float (order: 101);
single_float_optional: float (nullable, order: 101);
}
2 changes: 1 addition & 1 deletion crates/re_types/source_hash.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# This is a sha256 hash for all direct and indirect dependencies of this crate's build script.
# It can be safely removed at anytime to force the build script to run again.
# Check out build.rs to see how it's computed.
7dfe285eaa19872ddde875b16aa511da2b0e1f9fbeed723fa7a86920a2cc1738
b0b0d5d6d0f4131e52331303a2c2dfdfa11e03936e97242ef897c2b158979273
Loading

1 comment on commit 670812d

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Rust Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.25.

Benchmark suite Current: 670812d Previous: d9686da Ratio
batch_points_arrow/encode_log_msg 95524 ns/iter (± 1322) 49034 ns/iter (± 134) 1.95

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.