Skip to content

Commit

Permalink
Hunting down zombie TODOs: script + clean up (#3405)
Browse files Browse the repository at this point in the history
Added script to hunt down zombie issues, and fixed all existing ones.

This does _not_ run on CI; run it manually yourself with:
```
./scripts/zombie_todos.py --token <GITHUB_TOKEN>
```
  • Loading branch information
teh-cmc authored Sep 22, 2023
1 parent 25843e7 commit 7b58035
Show file tree
Hide file tree
Showing 30 changed files with 174 additions and 60 deletions.
6 changes: 6 additions & 0 deletions .github/workflows/reusable_checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,12 @@ jobs:
run: |
./scripts/lint.py
# NOTE: We don't want spurious failures caused by issues being closed, so this does not run on CI,
# at least for the time being.
# - name: Check for zombie TODOs
# run: |
# ./scripts/zombie_todos.py --token ${{ secrets.GITHUB_TOKEN }}

- name: Check for too large files
run: |
./scripts/ci/check_large_files.sh
Expand Down
3 changes: 0 additions & 3 deletions crates/re_arrow_store/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,8 +371,6 @@ fn datastore_internal_repr() {
/// ```text
/// cargo test -p re_arrow_store -- --nocapture datastore_internal_repr
/// ```
//
// TODO(#1524): inline visualization once it's back to a manageable state
#[derive(Debug, Clone)]
pub struct IndexedTable {
/// The timeline this table operates in, for debugging purposes.
Expand Down Expand Up @@ -539,7 +537,6 @@ impl Default for IndexedBucketInner {
/// cargo test -p re_arrow_store -- --nocapture datastore_internal_repr
/// ```
//
// TODO(#1524): inline visualization once it's back to a manageable state
// TODO(#1807): timeless should be row-id ordered too then
#[derive(Debug, Clone)]
pub struct PersistentIndexedTable {
Expand Down
3 changes: 0 additions & 3 deletions crates/re_arrow_store/src/store_gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,6 @@ impl DataStore {
// datastructures should be able to purge themselves based solely off of
// [`DataStore::oldest_time_per_timeline`].
//
// TODO(#1803): The GC should be aware of latest-at semantics and make sure they are upheld
// when purging data.
//
// TODO(#1823): Workload specific optimizations.
pub fn gc(&mut self, options: GarbageCollectionOptions) -> (Deleted, DataStoreStats) {
re_tracing::profile_function!();
Expand Down
2 changes: 0 additions & 2 deletions crates/re_arrow_store/src/store_write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,8 +545,6 @@ impl IndexedBucket {
/// ```text
/// cargo test -p re_arrow_store -- --nocapture datastore_internal_repr
/// ```
//
// TODO(#1524): inline visualization once it's back to a manageable state
fn split(&self) -> Option<(TimeInt, Self)> {
let Self {
timeline,
Expand Down
2 changes: 1 addition & 1 deletion crates/re_components/src/load_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ pub fn data_cells_from_mesh_file_contents(
bytes: Vec<u8>,
format: crate::MeshFormat,
) -> Result<Vec<DataCell>, FromFileError> {
// TODO(#2788): mesh indicator
// TODO(#3354): mesh indicator
let mesh = crate::EncodedMesh3D {
format,
bytes: bytes.into(),
Expand Down
3 changes: 0 additions & 3 deletions crates/re_log_types/src/data_cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,9 +276,6 @@ impl DataCell {
// ---

/// Builds an empty `DataCell` from a native component type.
//
// TODO(#1595): do keep in mind there's a future not too far away where components become a
// `(component, type)` tuple kinda thing.
#[inline]
pub fn from_native_empty<C: Component>() -> Self {
Self::from_arrow_empty(C::name(), C::arrow_field().data_type)
Expand Down
2 changes: 0 additions & 2 deletions crates/re_renderer/src/point_cloud_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,6 @@ impl<'a> PointCloudBatchBuilder<'a> {
/// Will add all positions.
/// Missing radii will default to `Size::AUTO`.
/// Missing colors will default to white.
///
/// TODO(#957): Clamps number of points to the allowed per-builder maximum.
#[inline]
pub fn add_points(
mut self,
Expand Down
4 changes: 2 additions & 2 deletions crates/re_renderer/src/renderer/lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,13 +356,13 @@ const LINE_STRIP_TEXTURE_SIZE: u32 = 256; // 256 x 256 x vec2<u32> == 0.5MiB, 65
impl LineDrawData {
/// Total maximum number of line vertices per [`LineDrawData`].
///
/// TODO(#957): Get rid of this limit!.
/// TODO(#3076): Get rid of this limit!.
pub const MAX_NUM_VERTICES: usize =
(POSITION_TEXTURE_SIZE * POSITION_TEXTURE_SIZE - 2) as usize; // Subtract sentinels

/// Total maximum number of line strips per [`LineDrawData`].
///
/// TODO(#957): Get rid of this limit!.
/// TODO(#3076): Get rid of this limit!.
pub const MAX_NUM_STRIPS: usize = (LINE_STRIP_TEXTURE_SIZE * LINE_STRIP_TEXTURE_SIZE) as usize;

/// Transforms and uploads line strip data to be consumed by gpu.
Expand Down
2 changes: 1 addition & 1 deletion crates/re_renderer/src/renderer/point_cloud.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ const DATA_TEXTURE_SIZE: u32 = 2048; // 2ki x 2ki = 4 Mi = 80 MiB
impl PointCloudDrawData {
/// Maximum number of vertices per [`PointCloudDrawData`].
///
/// TODO(#957): Get rid of this limit!.
/// TODO(#3076): Get rid of this limit!.
pub const MAX_NUM_POINTS: usize = (DATA_TEXTURE_SIZE * DATA_TEXTURE_SIZE) as usize;

/// Transforms and uploads point cloud data to be consumed by gpu.
Expand Down
2 changes: 1 addition & 1 deletion crates/re_sdk/src/recording_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -711,7 +711,7 @@ impl RecordingStream {
)
});

// TODO(#1629): unsplit splats once new data cells are in
// TODO(#1893): unsplit splats once new data cells are in
let splatted = (!splatted.is_empty()).then(|| {
splatted.push(DataCell::from_native([InstanceKey::SPLAT]));
DataRow::from_cells(RowId::random(), timepoint, ent_path, 1, splatted)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ impl ViewContextSystem for TransformContext {
&current_tree.path,
&entity_db.data_store,
&time_query,
// TODO(#1988): See comment in transform_at. This is a workaround for precision issues
// TODO(#1025): See comment in transform_at. This is a workaround for precision issues
// and the fact that there is no meaningful image plane distance for 3D->2D views.
|_| 500.0,
&mut encountered_pinhole,
Expand Down Expand Up @@ -330,7 +330,7 @@ fn transform_at(
// Above calculation is nice for a certain kind of visualizing a projected image plane,
// but the image plane distance is arbitrary and there might be other, better visualizations!

// TODO(#1988):
// TODO(#1025):
// As such we don't ever want to invert this matrix!
// However, currently our 2D views require do to exactly that since we're forced to
// build a relationship between the 2D plane and the 3D world, when actually the 2D plane
Expand Down
4 changes: 2 additions & 2 deletions crates/re_space_view_spatial/src/parts/assets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,12 @@ impl ViewPartSystem for Asset3DPart {
std::iter::once(Mesh3D::name()).collect()
}

// TODO(#2788): use this instead
// TODO(#3354): use this instead
// fn archetype(&self) -> Vec<ComponentName> {
// Mesh3D::required_components().to_vec()
// }

// TODO(#2788): use this instead
// TODO(#3354): use this instead
// fn heuristic_filter(
// &self,
// _store: &re_arrow_store::DataStore,
Expand Down
2 changes: 1 addition & 1 deletion crates/re_space_view_spatial/src/ui_2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ fn setup_target_config(
gpu_bridge::viewport_resolution_in_pixels(egui_painter.clip_rect(), pixels_from_points);
anyhow::ensure!(resolution_in_pixel[0] > 0 && resolution_in_pixel[1] > 0);

// TODO(#1988):
// TODO(#1025):
// The camera setup is done in a way that works well with the way we inverse pinhole camera transformations right now.
// This has a lot of issues though, mainly because we pretend that the 2D plane has a defined depth.
// * very bad depth precision as we limit the depth range from 0 to focal_length_in_pixels
Expand Down
3 changes: 0 additions & 3 deletions crates/re_types/definitions/rerun/archetypes/points2d.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,8 @@ namespace rerun.archetypes;

// ---

// TODO(#2371): archetype IDL definitions must always be tables
// TODO(#2372): archetype IDL definitions must refer to objects of kind component
// TODO(#2373): `attr.rerun.component_required` implies `required`
// TODO(#2427): distinguish optional vs. recommended in language backends
// TODO(#2521): always derive debug & clone for rust backend

/// A 2D point cloud with positions and optional colors, radii, labels, etc.
///
Expand Down
2 changes: 1 addition & 1 deletion crates/re_types/source_hash.txt

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions crates/re_types_builder/src/codegen/cpp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ const DOC_COMMENT_SUFFIX_TOKEN: &str = "DOC_COMMENT_SUFFIX_TOKEN";
const ANGLE_BRACKET_LEFT_TOKEN: &str = "SYS_INCLUDE_PATH_PREFIX_TOKEN";
const ANGLE_BRACKET_RIGHT_TOKEN: &str = "SYS_INCLUDE_PATH_SUFFIX_TOKEN";
const HEADER_EXTENSION_TOKEN: &str = "HEADER_EXTENSION_TOKEN";
const TODO_TOKEN: &str = "TODO_TOKEN";

fn quote_comment(text: &str) -> TokenStream {
quote! { #NORMAL_COMMENT_PREFIX_TOKEN #text #NORMAL_COMMENT_SUFFIX_TOKEN }
Expand Down Expand Up @@ -67,10 +66,6 @@ fn string_from_token_stream(token_stream: &TokenStream, source_path: Option<&Utf
.replace(&format!("\" {DOC_COMMENT_SUFFIX_TOKEN:?}"), "\n")
.replace(&format!("{ANGLE_BRACKET_LEFT_TOKEN:?} \""), "<")
.replace(&format!("\" {ANGLE_BRACKET_RIGHT_TOKEN:?}"), ">")
.replace(
&format!("{TODO_TOKEN:?}"),
"\n// TODO(#2647): code-gen for C++\n",
)
.replace("< ", "<")
.replace(" >", ">")
.replace(" ::", "::");
Expand Down
2 changes: 1 addition & 1 deletion docs/code-examples/clear_recursive.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@
rr2.log(f"arrows/{i}", rr2.Arrows3D(vector, origins=origin, colors=color))

# Now clear all of them at once.
# TODO(#3268): `rr2.Clear.recursive()`
# TODO(cmc): `rr2.Clear.recursive()`
rr2.log("arrows", rr2.Clear(True))
2 changes: 1 addition & 1 deletion docs/code-examples/clear_simple.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@

# Now clear them, one by one on each tick.
for i in range(len(vectors)):
# TODO(#3268): `rr2.Clear.flat()`
# TODO(cmc): `rr2.Clear.flat()`
rr2.log(f"arrows/{i}", rr2.Clear(False))
3 changes: 1 addition & 2 deletions docs/code-examples/segmentation_image_simple.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,4 @@
# Assign a label and color to each class
rr2.log("/", rr2.AnnotationContext([(1, "red", (255, 0, 0)), (2, "green", (0, 255, 0))]))

# TODO(#2792): SegmentationImage archetype
rr.log_segmentation_image("image", np.array(image))
rr2.log("image", rr2.SegmentationImage(image))
2 changes: 0 additions & 2 deletions docs/content/reference/viewer/viewport.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,4 @@ Rerun distinguishes various categories of Space Views:

Which category is used is determined upon creation of a Space View.

[TODO(#1164)](https://github.com/rerun-io/rerun/issues/1164): Allow configuring the category of a space view after its creation.

The kind of Space View determines which Entities it can display, how it displays them and the way they can be interacted with.
12 changes: 6 additions & 6 deletions examples/python/arkit_scenes/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def log_annotated_bboxes(annotation: dict[str, Any]) -> tuple[npt.NDArray[np.flo
Logs annotated oriented bounding boxes to Rerun.
We currently calculate and return the 3D bounding boxes keypoints, labels, and colors for each object to log them in
each camera frame TODO(#1581): once resolved this can be removed.
each camera frame TODO(#3412): once resolved this can be removed.
annotation json file
| |-- label: object name of bounding box
Expand All @@ -55,7 +55,7 @@ def log_annotated_bboxes(annotation: dict[str, Any]) -> tuple[npt.NDArray[np.flo
bbox_labels = []
num_objects = len(annotation["data"])
# Generate a color per object that can be reused across both 3D obb and their 2D projections
# TODO(#1581, #1728): once resolved this can be removed
# TODO(#3412, #1728): once resolved this can be removed
color_positions = np.linspace(0, 1, num_objects)
colormap = plt.colormaps["viridis"]
colors = [colormap(pos) for pos in color_positions]
Expand Down Expand Up @@ -93,7 +93,7 @@ def compute_box_3d(
"""
Given obb compute 3d keypoints of the box.
TODO(#1581): once resolved this can be removed
TODO(#3412): once resolved this can be removed
"""
length, height, width = half_size.tolist()
center = np.reshape(transform, (-1, 3))
Expand Down Expand Up @@ -123,7 +123,7 @@ def log_line_segments(entity_path: str, bboxes_2d_filtered: npt.NDArray[np.float
|/ |/
1 -------- 0
TODO(#1581): once resolved this can be removed
TODO(#3412): once resolved this can be removed
:param bboxes_2d_filtered:
A numpy array of shape (8, 2), representing the filtered 2D keypoints of the 3D bounding boxes.
Expand Down Expand Up @@ -175,7 +175,7 @@ def project_3d_bboxes_to_2d_keypoints(
"""
Returns 2D keypoints of the 3D bounding box in the camera view.
TODO(#1581): once resolved this can be removed
TODO(#3412): once resolved this can be removed
Args:
bboxes_3d: (nObjects, 8, 3) containing the 3D bounding box keypoints in world frame.
camera_from_world: Tuple containing the camera translation and rotation_quaternion in world frame.
Expand Down Expand Up @@ -242,7 +242,7 @@ def log_camera(
intrinsic = np.array([[fx, 0, cx], [0, fy, cy], [0, 0, 1]])
camera_from_world = poses_from_traj[frame_id]

# TODO(#1581): once resolved this can be removed
# TODO(#3412): once resolved this can be removed
# Project 3D bounding boxes into 2D image
bboxes_2d = project_3d_bboxes_to_2d_keypoints(bboxes, camera_from_world, intrinsic, img_width=w, img_height=h)
# clear previous centroid labels
Expand Down
2 changes: 0 additions & 2 deletions rerun_py/docs/gen_common_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,6 @@ def make_slug(s: str) -> str:


with mkdocs_gen_files.open(index_path, "w") as index_file:
# TODO(#1161): add links to our high-level docs!

# Hide the TOC for the index since it's identical to the left nav-bar
index_file.write(
"""---
Expand Down
2 changes: 1 addition & 1 deletion rerun_py/rerun_sdk/rerun/_baseclasses.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def as_component_batches(self) -> Iterable[ComponentBatchLike]:
for fld in fields(type(self)):
if "component" in fld.metadata:
comp = getattr(self, fld.name)
# TODO(#3341): Depending on what we decide
# TODO(#3381): Depending on what we decide
# to do with optional components, we may need to make this instead call `_empty_pa_array`
if comp is not None:
yield comp
Expand Down
4 changes: 2 additions & 2 deletions rerun_py/rerun_sdk/rerun/archetypes/clear.py

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions rerun_py/rerun_sdk/rerun/archetypes/segmentation_image.py

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions scripts/ci/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@
-r ../../rerun_py/requirements-lint.txt

Jinja2==3.1.2
Pillow==10.0.0
PyGithub==1.59.0

aiohttp==3.8.5
colorama==0.4.6
gitignore-parser==0.1.4
google-cloud-storage==2.9.0
packaging==23.1
python-frontmatter==1.0.0
requests>=2.31,<3
tomlkit==0.11.8
python-frontmatter==1.0.0
Pillow==10.0.0
2 changes: 2 additions & 0 deletions scripts/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -621,10 +621,12 @@ def main() -> None:
extensions = ["c", "cpp", "fbs", "h", "hpp" "html", "js", "md", "py", "rs", "sh", "toml", "txt", "wgsl", "yml"]

exclude_paths = {
"./.github/workflows/reusable_checks.yml", # zombie TODO hunting job
"./CODE_STYLE.md",
"./crates/re_types_builder/src/reflection.rs", # auto-generated
"./examples/rust/objectron/src/objectron.rs", # auto-generated
"./scripts/lint.py", # we contain all the patterns we are linting against
"./scripts/zombie_todos.py",
"./web_viewer/re_viewer.js", # auto-generated by wasm_bindgen
"./web_viewer/re_viewer_debug.js", # auto-generated by wasm_bindgen
}
Expand Down
Loading

0 comments on commit 7b58035

Please sign in to comment.