Merge/main to develop #4439
Merge/main to develop #4439
Conversation
…f classes or a dict mapping ids to labels
fix: coco category ids can now be not sequential - avoiding memory leak
Adding custom runs plugin and minor tweaks
DeepSort tracking enhancements
fix absolute css position for view bar icons
Add runs plugin to docs
* convert to timezone only if datetime is tzaware; add exposing test
* WIP 3D export fixes and tests * print * errant copypasta * move rewrite asset logic to scene_3d * cleanup 3d import/export tests a bit
Support non-sequential COCO category IDs
* add np casting to json encoder * isinstance * add test * add await
tweak zoom and workspace right alignment [v0.24.0]
* handle state payload in useRefresh * cleanup * skip e2e refresh * cleanup, comment
* fix multi pcd e2e * fix media visibility toggler e2e
release note cleanup
add release notes for Teams v1.6.1
* bug: use name arg in lights * add null checking when computing camera props * upgrade three.js deps * default name arg to class name in base class
[DOCS] Clarifying that segmentation/heatmap paths must be absolute
Fix YOLO-NAS inference
quickstart 3d dataset
Avoid slow session launch
FiftyOne Teams 1.7/FiftyOne v0.24.0 release notes
Merge/v0.24.0 to main
WalkthroughThis update introduces significant enhancements across various components, focusing on improving grid configuration, 3D visualization, and dataset management. Key changes include new state management for grid properties, updated projection metadata, refined detection overlays, and expanded dataset documentation. Additionally, the update includes performance optimizations, bug fixes, and new features like customizable user roles and 3D dataset support. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range and nitpick comments (15)
app/packages/looker/src/worker/threed-label-processor.ts (1)
111-165: The logic for determining the projection plane based on the normal vector and the subsequent calculation of projected corners is well-implemented. However, consider adding more comments to explain the logic behind the projection plane determination and the impact of different normals on the projection results. This will improve maintainability and understandability of the code.fiftyone/core/threed/scene_3d.py (1)
116-138: The example usage provided in the documentation is clear and demonstrates how to use theSceneclass effectively. This is helpful for new users who are integrating 3D scene management into their projects. Consider adding a bit more detail on the role of each method called in the example for better clarity.fiftyone/operators/executor.py (3)
Line range hint
160-160: Avoid using bareexceptstatements.Using bare
exceptstatements can catch unexpected exceptions, which can obscure underlying bugs. Specify the exception type to improve error handling.- except: + except Exception as e:
Line range hint
365-365: Unused local variablee.The variable
eis assigned but never used, which is unnecessary and can be removed for cleaner code.- except Exception as e: + except Exception:
Line range hint
886-886: Use direct boolean checks instead of comparing toTrue.Instead of comparing a boolean to
True, you can use the value directly in conditions. This makes the code cleaner and more Pythonic.- if not error.custom == True: + if not error.custom:fiftyone/utils/utils3d.py (4)
440-440: Ensure that thenormalfield is properly documented to reflect its usage and default behavior.Consider enhancing the documentation for the
normalfield to clearly state that it defaults to[0, 0, 1]if not specified. This helps in understanding the expected behavior without diving into the code.
Line range hint
538-610: Review the implementation of padding incompute_orthographic_projection_images.The padding calculation seems overly complex and could be simplified. Additionally, the function is quite long and handles multiple responsibilities. Consider refactoring to improve readability and maintainability.
- padding (None): a relative padding(s) in ``[0, 1]]`` to apply to the - field of view bounds prior to projection. Can either be a single - value to apply in all directions or a ``[padx, pady, padz]``. For - example, pass ``padding=0.25`` with no ``bounds`` to project onto - a tight crop of each point cloud with 25% padding around it + padding (None): a relative padding in ``[0, 1]`` to apply uniformly in all directions. For + example, pass ``padding=0.25`` to apply 25% padding around the bounds.Consider breaking down the function into smaller, more focused sub-functions to handle specific parts of the projection process.
Line range hint
692-731: Validate thepaddingparameter incompute_orthographic_projection_image.The function accepts a padding parameter that can be a list or a single value, but there's no validation to ensure the input is within the expected format or range. Adding input validation can prevent runtime errors and ensure the function behaves as expected.
+ if isinstance(padding, list) and len(padding) != 3: + raise ValueError("Padding must be a single float or a list of three floats.") + if isinstance(padding, float) and not (0 <= padding <= 1): + raise ValueError("Padding value must be between 0 and 1.")
Line range hint
847-903: Optimize the padding and bounds calculation in_parse_point_cloud.The padding and bounds calculation can be optimized and made more clear. Consider extracting this logic into a separate method to improve modularity and readability.
- if padding is not None: - pad = 0.5 * np.asarray(padding) * (max_bound - min_bound) - min_bound -= pad - max_bound += pad + def apply_padding(bounds, padding): + if padding is not None: + pad = 0.5 * np.asarray(padding) * (bounds[1] - bounds[0]) + bounds = (bounds[0] - pad, bounds[1] + pad) + return bounds + min_bound, max_bound = apply_padding((min_bound, max_bound), padding)fiftyone/core/session/session.py (5)
Line range hint
27-27: Replace the bareexceptwith a specific exception or handle the exception more gracefully.- except: + except ImportError:
Line range hint
527-527: Replace the bareexceptwith a specific exception or handle the exception more gracefully.- except: + except Exception as e: + logger.error("An error occurred: %s", str(e))
Line range hint
1149-1151: Replace lambda expressions withdeffunctions for better readability and debugging.- on_close_session: t.Callable[[CloseSession], None] = lambda _: setattr( - session, "_wait_closed", True - ) + def on_close_session(_: CloseSession) -> None: + setattr(session, "_wait_closed", True)Also applies to: 1154-1156, 1159-1161, 1164-1166, 1169-1171, 1174-1176, 1179-1181, 1186-1192, 1195-1199
Line range hint
1234-1234: Consider usingimportlib.util.find_specto check for the availability offiftyone.desktopinstead of importing it directly.- import fiftyone.desktop + from importlib import util + if util.find_spec("fiftyone.desktop") is not None: + import fiftyone.desktop + else: + raise ImportError("fiftyone.desktop is not installed")
Line range hint
1299-1299: Replace bareexceptwith a specific exception or handle the exception more gracefully.- except: + except Exception as e: + logger.error("An error occurred while writing JSON: %s", str(e))Also applies to: 1309-1309
fiftyone/zoo/datasets/base.py (1)
Line range hint
636-636: Refactor to avoid bareexceptstatements.Using bare
exceptstatements can catch unexpected exceptions and obscure programming errors. It's recommended to catch specific exceptions. Here's a suggested refactor:- except: + except Exception as e: + logger.error("An error occurred: %s", str(e))Also applies to: 643-643, 3360-3360, 3368-3368, 3376-3376, 3383-3383
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (17)
app/packages/looker/package.jsonis excluded by!**/*.jsonapp/yarn.lockis excluded by!**/*.lock,!**/*.lockdocs/source/images/app/app-empty.gifis excluded by!**/*.gif,!**/*.gifdocs/source/images/app/app-empty.pngis excluded by!**/*.png,!**/*.pngdocs/source/images/app/app-expanded.gifis excluded by!**/*.gif,!**/*.gifdocs/source/images/app/app-filters.gifis excluded by!**/*.gif,!**/*.gifdocs/source/images/app/app-scroll.gifis excluded by!**/*.gif,!**/*.gifdocs/source/images/app/app-scrollable-tooltip.gifis excluded by!**/*.gif,!**/*.gifdocs/source/images/app/app-views1.gifis excluded by!**/*.gif,!**/*.gifdocs/source/images/dataset_zoo/quickstart-3d.pngis excluded by!**/*.png,!**/*.pngdocs/source/images/datasets/quickstart-3d.gifis excluded by!**/*.gif,!**/*.gifdocs/source/images/datasets/quickstart-groups.gifis excluded by!**/*.gif,!**/*.gifdocs/source/images/datasets/quickstart-video.gifis excluded by!**/*.gif,!**/*.gifdocs/source/images/datasets/quickstart.gifis excluded by!**/*.gif,!**/*.gifdocs/source/images/teams/admin_roles_page.pngis excluded by!**/*.png,!**/*.pnge2e-pw/src/oss/specs/3d/fo3d-pcd-stl.spec.ts-snapshots/scene-chromium-darwin.pngis excluded by!**/*.png,!**/*.pnge2e-pw/src/oss/specs/3d/fo3d-pcd-stl.spec.ts-snapshots/scene-chromium-linux.pngis excluded by!**/*.png,!**/*.png
Files selected for processing (29)
- app/packages/components/src/components/PopoutSectionTitle/PopoutSectionTitle.tsx (1 hunks)
- app/packages/looker-3d/src/action-bar/ToggleGridHelper.tsx (1 hunks)
- app/packages/looker-3d/src/action-bar/shared.tsx (1 hunks)
- app/packages/looker-3d/src/action-bar/style.module.css (1 hunks)
- app/packages/looker-3d/src/fo3d/Gizmos.tsx (5 hunks)
- app/packages/looker-3d/src/fo3d/point-cloud/Pcd.tsx (2 hunks)
- app/packages/looker-3d/src/fo3d/point-cloud/use-pcd-material.tsx (1 hunks)
- app/packages/looker-3d/src/hooks/use-pcd-material-controls.ts (1 hunks)
- app/packages/looker-3d/src/state.ts (1 hunks)
- app/packages/looker/src/overlays/detection.ts (2 hunks)
- app/packages/looker/src/state.ts (1 hunks)
- app/packages/looker/src/worker/label-3d-projection-utils.test.ts (1 hunks)
- app/packages/looker/src/worker/label-3d-projection-utils.ts (1 hunks)
- app/packages/looker/src/worker/threed-label-processor.ts (5 hunks)
- docs/source/release-notes.rst (2 hunks)
- docs/source/teams/roles_and_permissions.rst (2 hunks)
- docs/source/user_guide/app.rst (3 hunks)
- docs/source/user_guide/dataset_zoo/datasets.rst (2 hunks)
- docs/source/user_guide/using_datasets.rst (12 hunks)
- e2e-pw/scripts/generate-screenshots-docker-image/build-docker-image.sh (1 hunks)
- fiftyone/core/labels.py (1 hunks)
- fiftyone/core/session/session.py (3 hunks)
- fiftyone/core/threed/scene_3d.py (6 hunks)
- fiftyone/operators/executor.py (1 hunks)
- fiftyone/utils/super_gradients.py (4 hunks)
- fiftyone/utils/utils3d.py (13 hunks)
- fiftyone/zoo/datasets/base.py (2 hunks)
- setup.py (1 hunks)
- tests/unittests/utils3d_tests.py (2 hunks)
Files skipped from review due to trivial changes (5)
- app/packages/components/src/components/PopoutSectionTitle/PopoutSectionTitle.tsx
- docs/source/user_guide/dataset_zoo/datasets.rst
- docs/source/user_guide/using_datasets.rst
- fiftyone/core/labels.py
- setup.py
Additional Context Used
ShellCheck (1)
e2e-pw/scripts/generate-screenshots-docker-image/build-docker-image.sh (1)
[info] 24-24: Use ./glob or -- glob so names with dashes won't become options.
Ruff (31)
fiftyone/core/session/session.py (14)
27-27: Do not use bare
except
527-527: Do not use bare
except
1149-1151: Do not assign a
lambdaexpression, use adef
1154-1156: Do not assign a
lambdaexpression, use adef
1159-1161: Do not assign a
lambdaexpression, use adef
1164-1166: Do not assign a
lambdaexpression, use adef
1169-1171: Do not assign a
lambdaexpression, use adef
1174-1176: Do not assign a
lambdaexpression, use adef
1179-1181: Do not assign a
lambdaexpression, use adef
1186-1192: Do not assign a
lambdaexpression, use adef
1195-1199: Do not assign a
lambdaexpression, use adef
1234-1234:
fiftyone.desktopimported but unused; consider usingimportlib.util.find_specto test for availability
1299-1299: Do not use bare
except
1309-1309: Do not use bare
exceptfiftyone/operators/executor.py (6)
160-160: Do not use bare
except
271-271: Do not use bare
except
278-278: Do not use bare
except
365-365: Local variable
eis assigned to but never used
392-392: Local variable
eis assigned to but never used
886-886: Avoid inequality comparisons to
True; useif not error.custom:for false checksfiftyone/utils/super_gradients.py (5)
16-16: Module level import not at top of file
16-16:
torchimported but unused
17-17: Module level import not at top of file
132-132: Ambiguous variable name:
l
136-136: Ambiguous variable name:
lfiftyone/zoo/datasets/base.py (6)
636-636: Do not use bare
except
643-643: Do not use bare
except
3360-3360: Do not use bare
except
3368-3368: Do not use bare
except
3376-3376: Do not use bare
except
3383-3383: Do not use bare
except
Biome (53)
app/packages/looker-3d/src/action-bar/ToggleGridHelper.tsx (3)
5-6: The default import is only used as a type.
118-118: This hook does not specify all of its dependencies: isFirstLoad
118-118: This hook does not specify all of its dependencies: setIsGridOn
app/packages/looker-3d/src/action-bar/shared.tsx (1)
1-2: The default import is only used as a type.
app/packages/looker-3d/src/fo3d/Gizmos.tsx (2)
123-123: This hook does not specify all of its dependencies: setCellSize
123-123: This hook does not specify all of its dependencies: setSectionSize
app/packages/looker-3d/src/fo3d/point-cloud/Pcd.tsx (2)
3-4: All these imports are only used as types.
5-6: All these imports are only used as types.
app/packages/looker-3d/src/fo3d/point-cloud/use-pcd-material.tsx (4)
26-26: Unexpected any. Specify a different type.
1-2: All these imports are only used as types.
9-10: All these imports are only used as types.
71-71: This hook specifies more dependencies than necessary: geometry
app/packages/looker-3d/src/hooks/use-pcd-material-controls.ts (2)
1-2: All these imports are only used as types.
11-12: All these imports are only used as types.
app/packages/looker-3d/src/state.ts (3)
1-1: All these imports are only used as types.
4-5: All these imports are only used as types.
5-6: All these imports are only used as types.
app/packages/looker/src/overlays/detection.ts (8)
195-195: The assignment should not be in an expression.
204-204: The assignment should not be in an expression.
292-301: Prefer for...of instead of forEach.
6-7: All these imports are only used as types.
7-8: All these imports are only used as types.
9-10: Some named imports are only used as types.
194-194: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
201-201: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
app/packages/looker/src/state.ts (10)
120-120: Unexpected any. Specify a different type.
129-129: This enum declaration contains members that are implicitly initialized.
187-187: Unexpected any. Specify a different type.
322-322: Unexpected any. Specify a different type.
411-411: Unexpected any. Specify a different type.
493-493: Unexpected any. Specify a different type.
4-5: All these imports are only used as types.
5-6: All these imports are only used as types.
6-7: All these imports are only used as types.
8-9: All these imports are only used as types.
app/packages/looker/src/worker/label-3d-projection-utils.test.ts (1)
1-2: Some named imports are only used as types.
app/packages/looker/src/worker/threed-label-processor.ts (17)
147-147: Declare variables separately
147-147: This variable implicitly has the any type.
201-203: Prefer for...of instead of forEach.
1-1: Some named imports are only used as types.
3-4: All these imports are only used as types.
4-5: All these imports are only used as types.
5-9: Some named imports are only used as types.
37-37: Use Number.Infinity instead of the equivalent global.
38-38: Use Number.Infinity instead of the equivalent global.
39-39: Use Number.Infinity instead of the equivalent global.
40-40: Use Number.Infinity instead of the equivalent global.
70-70: Use Number.Infinity instead of the equivalent global.
71-71: Use Number.Infinity instead of the equivalent global.
74-74: Use Number.Infinity instead of the equivalent global.
77-77: Use Number.Infinity instead of the equivalent global.
83-83: Use Number.Infinity instead of the equivalent global.
86-86: Use Number.Infinity instead of the equivalent global.
Path-based Instructions (12)
app/packages/looker-3d/src/action-bar/shared.tsx (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/looker/src/worker/label-3d-projection-utils.test.ts (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/looker-3d/src/fo3d/point-cloud/Pcd.tsx (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/looker/src/worker/label-3d-projection-utils.ts (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/looker-3d/src/state.ts (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/looker-3d/src/hooks/use-pcd-material-controls.ts (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/looker-3d/src/fo3d/point-cloud/use-pcd-material.tsx (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/looker-3d/src/action-bar/ToggleGridHelper.tsx (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/looker-3d/src/fo3d/Gizmos.tsx (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/looker/src/worker/threed-label-processor.ts (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/looker/src/overlays/detection.ts (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/looker/src/state.ts (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.
Learnings (1)
app/packages/looker-3d/src/fo3d/Gizmos.tsx (1)
User: sashankaryal PR: voxel51/fiftyone#4310 File: app/packages/looker-3d/src/fo3d/Gizmos.tsx:110-0 Timestamp: 2024-04-25T21:03:45.341Z Learning: The grid rendering logic in `Gizmos.tsx` is part of an external component and not directly modifiable within the project's codebase.
Additional comments not posted (29)
app/packages/looker-3d/src/action-bar/shared.tsx (1)
4-15: Refactor to useforwardRefcorrectly enhances component reusability.The use of
forwardRefallows theActionPopOvercomponent to accept a ref that can be forwarded to a DOM element, which is useful for managing focus, selection, or animations.app/packages/looker/src/worker/label-3d-projection-utils.test.ts (1)
4-22: Comprehensive unit tests forprojectTo2Dfunction.The tests cover projections to all three planes (xz, xy, yz), ensuring that the function behaves as expected across different scenarios.
e2e-pw/scripts/generate-screenshots-docker-image/build-docker-image.sh (2)
24-28: Improve rsync command safety by preventing potential option interpretation.
[REFACTOR_SUGGESTion]- rsync -aq "$FIFTYONE_ROOT_DIR"/* --exclude-from '.gitignore' \ + rsync -aq -- "$FIFTYONE_ROOT_DIR"/* --exclude-from '.gitignore' \Adding
--before specifying the source directory ensures that filenames starting with a dash are not interpreted as options, enhancing the script's robustness.
32-32: Docker image build step added to the script.This step is crucial for ensuring that the Docker environment is prepared with the latest changes, facilitating consistent testing across environments.
app/packages/looker-3d/src/action-bar/style.module.css (1)
38-64: Enhanced CSS for grid configuration layout.The use of flexbox in
.gridConfigContainerand.gridLabelimproves the layout's responsiveness and alignment, which is beneficial for maintaining a clean and scalable UI.app/packages/looker-3d/src/fo3d/point-cloud/Pcd.tsx (1)
35-39: Address potential issues with cloning inuseLoader.The comment indicates a workaround for a cloning issue. It's important to verify if this issue has been resolved in the library to potentially simplify the code.
app/packages/looker/src/worker/label-3d-projection-utils.ts (1)
19-83: Enhanced utility functions for 3D to 2D projections.The functions
rotatePointandgetProjectedCornersare well-implemented, providing robust tools for handling 3D geometries and their projection to 2D planes, which are essential for accurate rendering and analysis in 3D visualization contexts.app/packages/looker-3d/src/state.ts (1)
60-63: Ensure proper initialization and usage of Recoil atoms.All newly introduced atoms (
gridCellSizeAtom,gridSectionSizeAtom,isGridInfinitelyLargeAtom,gridSizeAtom,shouldGridFadeAtom) are correctly defined with appropriate defaults and keys. Make sure these atoms are used consistently across the application.Also applies to: 65-68, 70-78, 80-83, 86-94
fiftyone/utils/super_gradients.py (1)
52-52: Ensure correct model configuration for YOLO-NAS.The configuration for the YOLO-NAS model appears to be set correctly. This change should enable the use of YOLO-NAS models within the FiftyOne framework effectively.
app/packages/looker-3d/src/fo3d/point-cloud/use-pcd-material.tsx (1)
112-112: Ensure thekeyprop is used correctly in the<RgbShader>component.The addition of the
keyprop to the<RgbShader>component is a good practice to ensure that React correctly reconciles the list of components when the state changes. This change should help in maintaining performance and correctness.app/packages/looker-3d/src/action-bar/ToggleGridHelper.tsx (1)
20-110: Review the implementation of theGridConfiguratorandToggleGridHelpercomponents.The
GridConfiguratorandToggleGridHelpercomponents are well-implemented with clear and concise logic for handling grid configuration and toggling. The use of Recoil for state management in these components is appropriate and follows best practices.app/packages/looker-3d/src/fo3d/Gizmos.tsx (1)
141-165: Ensure the grid fading logic is correctly implemented.The grid fading logic, including the calculation of fade distance and strength, is implemented correctly. This feature enhances the visual quality of the grid by making it less obtrusive at greater distances.
app/packages/looker/src/worker/threed-label-processor.ts (2)
Line range hint
72-89: The addition of a third dimension to themin_boundandmax_boundfields, and the introduction of anormalfield inOrthogrpahicProjectionMetadataare significant changes that align with the PR's objectives to enhance 3D visualization capabilities. Ensure that all dependent functionalities are updated to handle these new dimensions correctly.
167-171: The implementation of the convex hull calculation using themonotone-convex-hull-2dlibrary is a good choice for efficiency. However, ensure that thenewProjectedCornersarray always contains at least three points before callingch, as the convex hull algorithm requires at least three points to compute a meaningful result.Verification successful
The
cornersarray is populated with the eight corners of a 3D bounding box, ensuring thattransformedCorners,projectedCorners, and consequentlynewProjectedCornerswill always have at least eight points. Therefore, the convex hull calculation using themonotone-convex-hull-2dlibrary is safe and will always have sufficient points to compute a meaningful result.
cornersis defined with eight points.transformedCornersis derived fromcorners.projectedCornersis derived fromtransformedCorners.newProjectedCornersis derived fromprojectedCorners.Thus,
newProjectedCornerswill always contain at least eight points.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that newProjectedCorners always contains at least three points. # Test: Search for the function usage. Expect: At least three points in newProjectedCorners. rg --type python $'newProjectedCorners.length >= 3'Length of output: 83
Script:
#!/bin/bash # Description: Verify that newProjectedCorners always contains at least three points. # Search for the definition and usage of newProjectedCorners in TypeScript files. rg 'newProjectedCorners' --type ts # Search for conditions checking the length of newProjectedCorners. rg 'newProjectedCorners.length' --type tsLength of output: 446
Script:
#!/bin/bash # Description: Extract the relevant code around the definition of newProjectedCorners and projectedCorners. # Extract lines around the definition of newProjectedCorners. rg 'const newProjectedCorners' --type ts -A 10 -B 10 # Extract lines around the definition of projectedCorners. rg 'const projectedCorners' --type ts -A 10 -B 10Length of output: 3237
Script:
#!/bin/bash # Description: Extract the relevant code around the definition of transformedCorners. # Extract lines around the definition of transformedCorners. rg 'const transformedCorners' --type ts -A 10 -B 10Length of output: 1954
Script:
#!/bin/bash # Description: Extract the relevant code around the definition of corners. # Extract lines around the definition of corners. rg 'const corners' --type ts -A 10 -B 10Length of output: 2063
app/packages/looker/src/overlays/detection.ts (1)
233-254: The implementation offillRectFor3dto draw the convex hull of the projected corners is a robust addition for handling 3D bounding boxes. The use of transparency and double stroke width enhances the visibility of the convex hull. Ensure that theconvexHullarray is always correctly populated and validated before this method is called to avoid runtime errors.docs/source/teams/roles_and_permissions.rst (1)
112-113: The clarification on how access levels are limited by user roles within groups is a valuable addition for understanding permission boundaries in FiftyOne Teams. This helps in setting clear expectations for role-based access control.app/packages/looker/src/state.ts (1)
68-70: The addition of a third dimension to themin_boundandmax_boundfields, and the introduction of anormalfield inOrthogrpahicProjectionMetadataare consistent with the changes inthreed-label-processor.ts. This ensures that the state management is aligned with the 3D data handling enhancements.fiftyone/core/threed/scene_3d.py (1)
386-393: The method for loading a scene from an.fo3dfile is correctly implemented with appropriate error handling for file extension validation. This ensures that the function behaves predictably and errors are caught early.tests/unittests/utils3d_tests.py (2)
288-288: Added assertion for thenormalfield inOrthographicProjectionMetadata.This ensures that the new field is correctly serialized and deserialized, maintaining data integrity across operations.
308-308: Added assertion for thenormalfield in the dataset sample.This is a good practice to ensure that the new field is correctly handled in practical scenarios, not just theoretically in the metadata class.
fiftyone/operators/executor.py (3)
Line range hint
271-271: Avoid using bareexceptstatements.
Line range hint
278-278: Avoid using bareexceptstatements.
Line range hint
392-392: Unused local variablee.fiftyone/core/session/session.py (1)
1322-1331: The function_pull_group_sliceis a new addition and correctly handles the optional parameters fordatasetandview. It uses the propertiesgroup_slicefrom bothdatasetandviewto determine the appropriate return value.docs/source/user_guide/app.rst (2)
56-59: Updated image for the FiftyOne App documentation.This change updates the image to presumably a more current or relevant screenshot, which should help users better understand the app's interface.
1094-1107: Added tooltip customization instructions.This addition provides users with guidance on how to customize tooltips in the FiftyOne App, enhancing the documentation's utility by addressing a specific user interaction with the app.
fiftyone/zoo/datasets/base.py (1)
3182-3204: Add a new class for a small 3D dataset.The addition of
Quickstart3DDatasetclass provides a small 3D dataset with meshes, point clouds, and oriented bounding boxes, which is a valuable resource for quick demonstrations or testing of 3D data handling capabilities.docs/source/release-notes.rst (2)
6-135: The release notes are well-documented and provide a comprehensive overview of the updates in FiftyOne Teams 1.7.0 and FiftyOne 0.24.0. The inclusion of pull request links enhances traceability.
2713-2714: The additions to the release notes are clear and concise. Please ensure that the reference links are correctly linked and functional.Verification successful
The reference links in the release notes are correctly linked and functional.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the reference links in the release notes are correctly linked and functional. # Test: Search for the reference links in the documentation. Expect: Correctly linked and functional references. rg --type rst $':ref:'Length of output: 215573
| normal (None): the normal vector of the plane onto which the projection | ||
| was performed. If not specified, ``[0, 0, 1]`` is assumed |
There was a problem hiding this comment.
Add a default value for the normal field in OrthographicProjectionMetadata.
- normal (None): the normal vector of the plane onto which the projection
- was performed. If not specified, ``[0, 0, 1]`` is assumed
+ normal (None): the normal vector of the plane onto which the projection
+ was performed. If not specified, ``[0, 0, 1]`` is assumed
+ normal = fof.ListField(fof.FloatField(), default=[0, 0, 1])Committable suggestion was skipped due low confidence.
| // max point size is arbitrary. ideally, we should also offer a text box for users to input their desired point size | ||
| max: 50, |
There was a problem hiding this comment.
Consider adding a user input for point size customization.
The comment on line 37 suggests adding a text box for user input to set the point size. This would enhance user interaction by allowing them to specify a custom point size. Consider implementing this feature to improve usability.
| @@ -1,11 +1,15 @@ | |||
| import { ActionPopOverDiv, ActionPopOverInner } from "../containers"; | |||
| import React, { forwardRef } from "react"; | |||
There was a problem hiding this comment.
Consider using type-only imports for React.
- import React, { forwardRef } from "react";
+ import type { ForwardRefRenderFunction } from "react";
+ import { forwardRef } from "react";This change ensures that React is only imported for its type definitions where needed, aligning with best practices for cleaner and more efficient code.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| import React, { forwardRef } from "react"; | |
| import type { ForwardRefRenderFunction } from "react"; | |
| import { forwardRef } from "react"; |
| import { describe, expect, it } from "vitest"; | ||
| import { Vec3, projectTo2D } from "./label-3d-projection-utils"; |
There was a problem hiding this comment.
Consider using type-only imports for test utilities.
- import { describe, expect, it } from "vitest";
+ import type { Describe, Expect, It } from "vitest";
+ import { describe, expect, it } from "vitest";This change ensures that test utilities are only imported for their type definitions where needed, which can help in reducing the final bundle size if tree-shaking is not effectively configured.
Committable suggestion was skipped due low confidence.
What changes are proposed in this pull request?
Merge Main to Develop after v0.24.0 release
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Documentation
Tests