TODO hunting

crates/re_components/src/class_id.rs

@@ -1,7 +1,5 @@
 use arrow2_convert::{ArrowDeserialize, ArrowField, ArrowSerialize};
 
-// TODO: explain why we keep that one (needed for annotation context)
-
 /// A 16-bit ID representing a type of semantic class.
 ///
 /// Used to look up a [`crate::context::ClassDescription`] within the [`crate::context::AnnotationContext`].

crates/re_components/src/color.rs

@@ -1,5 +1,3 @@
-// TODO: explain why we keep that one (needed for annotation context)
-
 /// An RGBA color tuple with unmultiplied/separate alpha,
 /// in sRGB gamma space with linear alpha.
 ///

crates/re_components/src/label.rs

@@ -1,7 +1,5 @@
 use arrow2_convert::{ArrowDeserialize, ArrowField, ArrowSerialize};
 
-// TODO: explain why we keep that one (needed for annotation context)
-
 /// A String label component
 ///
 /// ```

crates/re_components/src/lib.rs

@@ -52,11 +52,12 @@ pub mod datagen;
 // ----------------------------------------------------------------------------
 // TODO(emilk): split into modules, like we do in re_sdk/src/lib.rs
 
-// TODO: stuff we keep for annotation context
-pub(crate) use self::{class_id::LegacyClassId, keypoint_id::LegacyKeypointId, label::LegacyLabel};
-
-// TODO: has to be fully public cause we have something weird
-pub use self::color::LegacyColor;
+// NOTE: We keep these legacy types around because they are used by the legacy `AnnotationContext`,
+// which needs them to implement `arrow2-convert`'s' traits.
+// TODO(#2794): get rid of this once `AnnotationContext` has been migrated.
+pub(crate) use self::{
+    class_id::LegacyClassId, color::LegacyColor, keypoint_id::LegacyKeypointId, label::LegacyLabel,
+};
 
 pub use self::{
     arrow::Arrow3D,

crates/re_log_types/src/component.rs

@@ -18,7 +18,10 @@ pub trait LegacyComponent: ArrowField {
     }
 }
 
-// TODO: explain
+// NOTE: We have a ton of legacy tests that rely on the old APIs and `Point2D`.
+// Since the new `Point2D` is binary compatible with the old we can easily drop the old one, but
+// for that we need the new one to implement the `LegacyComponent` trait.
+// TODO(cmc): remove once the migration is over
 impl LegacyComponent for re_types::components::Point2D {
     fn legacy_name() -> ComponentName {
         use re_types::Loggable as _;

crates/re_types/definitions/rerun/components/class_id.fbs

@@ -8,7 +8,7 @@ namespace rerun.components;
 
 // ---
 
-// TODO: serde should be cfg_attr gated here
+// TODO(#2801): serde should be cfg_attr gated here
 
 /// A 16-bit ID representing a type of semantic class.
 ///

crates/re_types/definitions/rerun/components/keypoint_id.fbs

@@ -8,7 +8,7 @@ namespace rerun.components;
 
 // ---
 
-// TODO: serde should be cfg_attr gated here
+// TODO(#2801): serde should be cfg_attr gated here
 
 /// A 16-bit ID representing a type of semantic keypoint within a class.
 ///

crates/re_types/src/components/point2d_ext.rs

@@ -60,7 +60,11 @@ impl From<Point2D> for glam::Vec3 {
     }
 }
 
-// TODO: explain
+// NOTE: We have a ton of legacy tests that rely on the old APIs and `Point2D`.
+// Since the new `Point2D` is binary compatible with the old we can easily drop the old one, but
+// for that we need the new one to implement the `LegacyComponent` trait, which means implementing
+// `ArrowField` to begin with!
+// TODO(cmc): remove once the migration is over
 impl arrow2_convert::field::ArrowField for Point2D {
     type Type = Self;
 

rerun_py/rerun_sdk/rerun/log/points.py

@@ -96,7 +96,7 @@ def log_point(
 
     if position is not None:
         if position.size == 2:
-            p = Points2D(
+            points2d = Points2D(
                 points=position,
                 radii=radius,
                 colors=color,
@@ -105,9 +105,9 @@ def log_point(
                 class_ids=class_id,
                 keypoint_ids=keypoint_id,
             )
-            return log_any(entity_path, p, ext=ext, timeless=timeless, recording=recording)
+            return log_any(entity_path, points2d, ext=ext, timeless=timeless, recording=recording)
         elif position.size == 3:
-            p = Points3D(
+            points3d = Points3D(
                 points=position,
                 radii=radius,
                 colors=color,
@@ -116,7 +116,7 @@ def log_point(
                 class_ids=class_id,
                 keypoint_ids=keypoint_id,
             )
-            return log_any(entity_path, p, ext=ext, timeless=timeless, recording=recording)
+            return log_any(entity_path, points3d, ext=ext, timeless=timeless, recording=recording)
         else:
             raise TypeError("Position must have a total size of 2 or 3")
 
@@ -213,8 +213,8 @@ def log_points(
 
     if positions.any():
         if positions.shape[1] == 2:
-            # TODO: but then we probably don't support empty positions anymore...
-            p = Points2D(
+            # TODO: cannot _not_ specify points anymore!
+            points2d = Points2D(
                 points=positions,
                 radii=radii,
                 colors=colors,
@@ -224,9 +224,9 @@ def log_points(
                 keypoint_ids=keypoint_ids,
                 instance_keys=identifiers_np,
             )
-            return log_any(entity_path, p, ext=ext, timeless=timeless, recording=recording)
+            return log_any(entity_path, points2d, ext=ext, timeless=timeless, recording=recording)
         elif positions.shape[1] == 3:
-            p = Points3D(
+            points3d = Points3D(
                 points=positions,
                 radii=radii,
                 colors=colors,
@@ -236,6 +236,6 @@ def log_points(
                 keypoint_ids=keypoint_ids,
                 instance_keys=identifiers_np,
             )
-            return log_any(entity_path, p, ext=ext, timeless=timeless, recording=recording)
+            return log_any(entity_path, points3d, ext=ext, timeless=timeless, recording=recording)
         else:
             raise TypeError("Positions should be Nx2 or Nx3")