Don't use builtin required anymore, introduce nullable instead

[teh-cmc]

Fixes #2525
Closes #2524

---

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)

• PR Build Summary
• Docs preview
• Examples preview

[emilk]

Good call! I also like that the default now is non-nullable.

Can we add a check for the required attribute, and if found error with a "fields are required by default. use nullable for non-required fields"?

[emilk]

minor nit: the name "is_nullable" for some reason makes me expect it to be a bool (order: 100, is_nullable: true), while the shorter "nullable" sounds more like a flag (order: 100, nullable)

[teh-cmc]

Agreed

[teh-cmc]

Good call! I also like that the default now is non-nullable.

Can we add a check for the required attribute, and if found error with a "fields are required by default. use nullable for non-required fields"?

Sh*t, I actually wanted to do this in this one but merged too quickly.

• Detect usage of required and panic with helpful message #2634

[abey79]

This PR appears to break TranslationRotationScale3D, which would benefit from having all its fields nullable:

include "arrow/attributes.fbs";
include "fbs/attributes.fbs";

include "./rotation3d.fbs";
include "./scale3d.fbs";
include "./vec3d.fbs";

namespace rerun.datatypes;

// ---

/// Representation of an affine transform via separate translation, rotation & scale.
table TranslationRotationScale3D (
  "attr.rust.derive": "Copy, PartialEq",
  order: 600
) {
  /// 3D translation vector, applied last.
  translation: rerun.datatypes.Vec3D (nullable, order: 100);

  /// 3D rotation, applied second.
  rotation: rerun.datatypes.Rotation3D (order: 200);

  /// 3D scale, applied first.
  scale: rerun.datatypes.Scale3D (order: 300);

  /// If true, the transform maps from the parent space to the space where the transform was logged.
  /// Otherwise, the transform maps from the space to its parent.
  from_parent: bool = false (order: 400);
}

Adding nullable to rotation and/or scale yields the following error:

thread 'main' panicked at 'nullable unions cannot be represented in Arrow, found one at "rerun.datatypes.TranslationRotationScale3D#scale"', /Users/hhip/src/rerun/crates/re_types_builder/src/objects.rs:137:21

That error feels just wrong though, from using the v1 API:

rr.log_transform3d("foo", rr.TranslationRotationScale3D())

Am I missing something?

[teh-cmc]

You're right, this should definitely be legal in this instance: while we cannot express nullability on the union itself, we can certainly piggyback on the validity bitmap of the parent struct here.

Come to think of it, since we always have a mandatory table layer first and foremost for our components, I'm not even sure there's any way one could end up with a nullable union using our IDL in the first place 🤔.

I'll have a deeper look on monday, in the meantime you should be fine if you just get rid of the check on your local copy, right?

[abey79]

I'll give it a try tomorrow 👍🏻 For now I've undertook the writing of a novel to describe what's going on with python codegen.