Release/v1.2.0 #5306

findtopher · 2024-12-20T13:02:12Z

What changes are proposed in this pull request?

release version 1.2.0

Summary by CodeRabbit

Release Notes

New Features
- Enhanced documentation for Data Lens integration with Databricks.
- New Error and ErrorIcon components for improved error handling in model evaluations.
- Instance segmentation support in dataset import/export processes.
- Added support for AWS session tokens in cloud credential configurations.
- Improved functionality in the EvaluationPanel class for handling binary classification and segmentation evaluations.
- New method for managing sidebar groups in the DatasetAppConfig class.
- New fetchWithLinearBackoff function for enhanced HTTP request handling.
Bug Fixes
- Improved error handling in the ImageElement and VideoElement classes.
- Fixed visibility checks in grid tagging tests.
- Addressed a bug that could lead to StopIteration errors during long-running operations with API connections.
Documentation
- Expanded sections in the user guide for instance segmentation and dynamic attributes.
- Updated API documentation to reflect new method signatures and functionalities.
- New release note sections for "FiftyOne Teams 2.3.0" and "FiftyOne Teams 2.2.0".
Tests
- Added unit tests for instance segmentation functionality and the TableView class.
- Enhanced test coverage for detection masks and histogram panels.
Chores
- Updated dependency versions in requirements/common.txt and setup.py.

bump mongoengine, motor, and pymongo

1.0.2 to develop

…canvas performance: use offscreen canvas to decode on-disk masks

Merge `release/v1.1.0` to `develop`

…into develop

Merge `release/v1.1.0` to `develop`

…into develop

Merge `release/v1.1.0` to `develop`

…into develop

* reset buffers to initial state, handle single frame * assert grid tagging with locators

* update font size when pulling from looker cache * add fontSize to attach

remove erroneous base url extraction

fix load_evaluation bug with serialized_info

[docs] update data lens documentation to include additional examples

coderabbitai · 2024-12-20T13:02:23Z

Walkthrough

This pull request introduces comprehensive changes across multiple components of the FiftyOne project, focusing on enhancing mask handling, improving error management, and refining various system utilities. The modifications span workflow configurations, core functionality, and testing infrastructure, with significant updates to how detection and segmentation masks are processed, stored, and referenced. Key areas of improvement include mask path handling, workflow optimization, and expanded support for different label types.

Changes

File/Group	Change Summary
`.github/workflows/`	Updated workflow configurations for build, documentation, end-to-end testing, and pull request processes
`app/packages/core/src/`	Enhanced component rendering, error handling, and routing logic
`fiftyone/core/labels.py`	Added new `_HasMedia` mixin, introduced mask path handling for Detection and Segmentation classes
`fiftyone/utils/`	Updated utility functions for handling masks, labels, and data import/export
`requirements/common.txt`	Updated dependency versions for mongoengine, motor, and pymongo
`setup.py`	Incremented package version to 1.2.0 and updated dependency specifications

Suggested labels

documentation

Suggested reviewers

benjaminpkane

Poem

🐰 Hop, hop, through the code we go,
Masks and paths now start to flow
Workflows dance, dependencies sing
FiftyOne spreads its digital wing! 🚀

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?

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

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 using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 12

🔭 Outside diff range comments (1)

fiftyone/utils/ultralytics.py (1)
Inconsistent mask access patterns found in the codebase

The verification reveals mixed usage of direct mask access and getter methods across the codebase:

Direct access (detection.mask) still present in:

fiftyone/utils/labels.py

fiftyone/utils/data/importers.py

fiftyone/core/labels.py (multiple instances)

Getter method (get_mask()) is used in:

fiftyone/utils/labelstudio.py

fiftyone/utils/labelbox.py

fiftyone/utils/eval/segmentation.py

fiftyone/utils/eta.py

Consider updating the remaining direct mask access patterns to use the getter method for consistency.

🔗 Analysis chain

Line range hint 599-599: LGTM! Verify consistent mask access patterns across the codebase.

The change from direct mask access to using get_mask() aligns with encapsulation best practices. This change is part of a broader effort to standardize mask handling across the codebase.
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Check for any remaining direct mask access patterns
# that might need to be updated for consistency

# Search for direct mask access patterns
rg -A 2 'detection\.mask\b|\.mask\s*='

# Search for the new getter method usage
rg -A 2 'get_mask'
Length of output: 4007

🧹 Nitpick comments (57)

app/packages/state/src/recoil/sidebar.ts (1)
764-773: LGTM! Consider adding JSDoc documentation.

The implementation efficiently uses Set for collapsed paths and correctly filters list fields based on unsupported filter types.

Add JSDoc documentation to explain the purpose and behavior of this selector:
+/**
+ * Selector that returns a Set of paths that should be collapsed in the sidebar.
+ * This includes dictionary fields and list fields with unsupported filter types.
+ * @returns {Set<string>} Set of paths to be collapsed
+ */
 export const collapsedPaths = selector<Set<string>>({
fiftyone/core/odm/dataset.py (2)

642-646: Potential duplication check is good, but consider performance in large lists
The loop removes the path from every group that contains it, ensuring each path appears only once. For large numbers of groups, a single pass is fine, but if performance becomes an issue, consider optimizing.

653-661: Insert logic is clear but may benefit from error handling
If “after_group” is a value not found in the list, the code appends at the end. This is valid logic, but consider logging a message so the user knows the insertion point couldn’t be located.

app/packages/looker/src/worker/decorated-fetch.ts (2)

1-4: Consider adding 429 (Too Many Requests) to the non-retryable list or special-casing it.
HTTP 429 indicates rate limits; depending on your requirements, you may wish to handle it with exponential backoff rather than immediate failure or ignoring it as retryable.

17-24: Validate default retry parameters against fetch constraints.
Your chosen default of 10 retries × 200ms each yields a maximum ~2 second delay, not counting request time. If the environment is prone to slow connections or API throttling, consider whether a more flexible strategy (like exponential backoff) is more suitable.

app/packages/looker/src/worker/canvas-decoder.ts (1)

53-71: Consider error logging or fallback for createImageBitmap issues.
In certain browser contexts, createImageBitmap might be unavailable or throw unexpected errors (e.g., memory constraints). Adding a guarded fallback approach or more detailed logs could enhance troubleshooting.

app/packages/looker/src/worker/disk-overlay-decoder.ts (1)

83-112: Ensure fetch fallback for overlay path.
If overlayImageUrl is invalid or missing, and fetching fails, code logs an error and returns. This is correct for skipping decoding. Consider whether you need partial or fallback overlays in some workflows.

app/packages/looker/src/overlays/heatmap.ts (1)

9-18: Keep consistent naming for color utility imports.
Imports like getColor, getRGBA, getRGBAColor, and get32BitColor are functionally separate yet thematically related. Ensure they remain consistent across all overlays for readability.

app/packages/looker/src/overlays/segmentation.ts (1)

260-266: Intuitive memory & resource cleanup
Introducing getSizeBytes() to estimate memory usage is very helpful for performance monitoring, and the cleanup method properly closes the bitmap resource.

Consider logging or otherwise surfacing the estimated size bytes for debugging or performance optimization in production.

app/packages/looker/src/worker/index.ts (5)

97-104: Straightforward, stepwise documentation
Providing a clear top-level comment describing each step in mask and overlay processing is extremely helpful for maintainers.

140-151: Robust decodeOverlayOnDisk usage
Accumulating decode promises (lines 140-151) centralizes the overlay decoding approach into one function. This is more modular and maintainable.

Consider adding error-handling or retry logic in decodeOverlayOnDisk for more resilience.

252-253: Helper for bitmap creation
Clear function naming and separation of concerns. This encourages reusability and simpler testing.

350-356: Accumulating partial results
You push new promises into imageBitmapPromises and buffers into maskTargetsBuffers. This matches your approach in processLabels.

Consider using spread operators carefully to avoid potential performance overhead in extremely large arrays.

Line range hint 485-509: Granular label filtering
This conditional logic ensures you can filter and flatten various classification/detection scenarios neatly. It’s fairly verbose, but logically consistent.

Consider extracting repeated filter steps to a helper function to reduce method size and improve readability.

fiftyone/operators/builtins/panels/model_evaluation/__init__.py (2)

9-9: Import usage
The usage of defaultdict and Counter from “collections” is straightforward and ideal for tallying or grouping metrics.

485-509: Complex multi-conditional logic
Although lengthy, the if-else structure is explicit for each scenario (class, matrix, field), making it straightforward, if somewhat repetitive.

Consider a small refactor or pattern-based approach to reduce repeated lines.
app/packages/looker/src/lookers/abstract.ts (4)
499-499: Font size update merges properly with existing options.

Looks good. The approach to pass fontSize via the options object is consistent. Validate possible overrides in the future for dynamic font resizing.

524-560: Detach buffer checks.

This logic to handle possibly detached ArrayBuffers for overlay data is quite helpful. Consider robust logging or error reporting whenever you skip an update so that diagnosing performance or data issues is easier.
 // check for detached buffer (happens if user is switching colors too fast)
+ // Optional: Add a debug log to help track data flow issues
 if (typeof buffer.detached !== "undefined") {
     ...
}
744-749: Cleaning up overlays remains straightforward.

This is an important housekeeping method. Just ensure that overlay cleanup is safe to call multiple times. It might be worth a defensive check to avoid potential double cleanup calls.

Line range hint 750-798: Handling dataCloneError.

The retry logic to send data without transferring buffers is a safety net. Implementing extremely granular logging or metrics about how often this path is triggered can help debug user experiences with detached buffers.
fiftyone/utils/labels.py (5)

214-225: Static analysis suggests combining branches.

While combining branches might reduce code duplication, ensure clarity doesn’t suffer. If the logic remains more readable in separate conditionals, keep them separate. Otherwise, a logical OR approach can reduce repetition.

🧰 Tools

🪛 Ruff (0.8.2)

214-225: Combine if branches using logical or operator

Combine if branches

(SIM114)

288-299: Consider merging if statements.

Same suggestion from static analysis about combining these if branches. Evaluate if that simplification remains readable without sacrificing clarity.

🧰 Tools

🪛 Ruff (0.8.2)

288-299: Combine if branches using logical or operator

Combine if branches

(SIM114)

309-310: Static analysis suggests merging nested checks.

Proceed carefully with combining if statements. It's often beneficial, but ensure that clarity is preserved.

🧰 Tools

🪛 Ruff (0.8.2)

309-310: Use a single if statement instead of nested if statements

(SIM102)

494-496: Handle overwrite carefully if output directory is large.

Deleting the entire output_dir can be destructive. Ensure the user’s aware or prompt for confirmation in a UI scenario.

Line range hint 513-560: Generating detections from segmentation masks with optional image exports.

Clear logic is used to convert segmentation data into detection labels, exporting instance masks if requested. Logging the file path for debugging might help diagnosing issues, but this is not mandatory.

app/packages/core/src/plugins/SchemaIO/components/NativeModelEvaluationView/Evaluation.tsx (1)

1147-1149: Difference column for class performance.

Consistent approach across all tables. Keep an eye on large differences and how they are displayed. Possibly consider highlighting significant changes more strongly.

fiftyone/core/labels.py (3)

44-47: Consider clarifying usage for _HasMedia.
The new "_HasMedia" mixin sets a pattern for classes that have a media path. It might be helpful to expand the docstring to indicate that if the subclass redefines "_MEDIA_FIELD", it should ensure consistency in any get/import/export methods.

448-462: Potential performance overhead when importing masks.
Repeated reading of large masks from disk can be costly if used frequently. If performance becomes an issue, consider caching or lazy loading the mask to reduce repeated I/O.

653-653: Verify the intended user experience of skipping detections without a mask.
The warning "Skipping detection(s) with no instance mask" might be beneficial to detail how many detections were skipped, or inform the user more comprehensively if missing masks are expected.
fiftyone/utils/data/exporters.py (4)
15-17: Unused imports or potential re-usage of imports.
Currently, "json_util" and "pydash" are imported together. If "json_util" is only used within certain sections or is replaceable, consider verifying it is needed. This is merely a suggestion to reduce potential clutter.

1897-1897: Renaming blacklist.
"blacklist" has recently been replaced with more descriptive terms like "denylist" in many contexts. Consider updating the parameter for clarity and modern conventions.

2047-2050: Apply ternary operator for cleaner code.
The static analysis suggests using a ternary expression for readability.

Use the following diff to refactor:
-if key is not None:
-    _value = _d.get(key, None)
-else:
-    _value = _d
+_value = _d.get(key, None) if key is not None else _d
🧰 Tools

🪛 Ruff (0.8.2)

2047-2050: Use ternary operator _value = _d.get(key, None) if key is not None else _d instead of if-else-block

Replace if-else-block with _value = _d.get(key, None) if key is not None else _d

(SIM108)

2355-2358: Replicate the same ternary operator improvement here.
Following the prior suggestion, you can likewise simplify this if-else to a one-liner.

Use the following diff to refactor:
-if key is not None:
-    _value = _d.get(key, None)
-else:
-    _value = _d
+_value = _d.get(key, None) if key is not None else _d
🧰 Tools

🪛 Ruff (0.8.2)

2355-2358: Use ternary operator _value = _d.get(key, None) if key is not None else _d instead of if-else-block

Replace if-else-block with _value = _d.get(key, None) if key is not None else _d

(SIM108)
app/packages/looker/src/lookers/utils.ts (1)

3-7: Validate frameNumber within buffers.
This function works as intended, but be mindful of negative or invalid frame numbers. Depending on usage, you could throw an error or return false if frameNumber < 0 or if buffers is empty.

app/packages/looker/src/worker/shared.ts (1)

14-21: Consider simplifying the switch statement.
Since there's only one specialized case (HEATMAP) and a default fallback, a simple dictionary lookup or an inline ternary might reduce code size. However, your current approach is perfectly readable and explicit.

app/packages/looker/src/lookers/utils.test.ts (1)

11-13: Add edge case tests for buffer boundaries

While the test covers basic scenarios, consider adding edge cases:

Test with empty buffers array

Test with overlapping buffer ranges

Test with single-frame buffers ([5,5])
app/packages/core/src/plugins/SchemaIO/components/NativeModelEvaluationView/Evaluate.tsx (1)
7-8: Enhance type safety for permissions object

Consider defining a specific type for permissions instead of using Record<string, boolean>.
-  permissions: Record<string, boolean>;
+  permissions: {
+    can_evaluate: boolean;
+    // add other permission flags as needed
+  };
app/packages/core/src/plugins/SchemaIO/components/NativeModelEvaluationView/Error.tsx (1)
38-40: Update type name to match component

Update the type name to match the renamed component.
-type ErrorProps = {
+type EvaluationErrorProps = {
   onBack: () => void;
 };
tests/unittests/operators/tableview_tests.py (2)
7-15: Add docstring to describe test cases

The test method would benefit from a docstring describing the test scenarios and expected outcomes.
 def test_table_view_basic(self):
+    """
+    Test basic TableView operations:
+    - Adding columns with unique names
+    - Verifying duplicate column prevention
+    - Testing row actions and tooltips
+    """
     table = TableView()
42-46: Consider adding edge case tests for tooltips

The current tests verify basic tooltip functionality. Consider adding edge cases such as:

Invalid row/column indices

Empty tooltip text

Special characters in tooltip text
e2e-pw/src/oss/specs/grid-tagging.spec.ts (1)
47-50: Extract repeated visibility checks into helper function

Both loops perform similar visibility checks. Consider extracting this logic into a reusable helper function to improve maintainability.
async function verifyGridItems(grid: GridPom, startIndex: number, endIndex: number, checkTag = false) {
  for (let i = startIndex; i <= endIndex; i++) {
    const locator = grid.locator.getByText(`/tmp/${i}.png`);
    await expect(locator).toBeVisible();
    if (checkTag) {
      await expect(
        locator.locator("..").getByTestId("tag-tags-grid-test")
      ).toBeVisible();
    }
  }
}
Usage:
await verifyGridItems(grid, 31, 54);
// After tagging
await verifyGridItems(grid, 31, 54, true);
Also applies to: 58-64
app/packages/spaces/src/components/AddPanelItem.tsx (2)
22-24: Consider using early return pattern

The conditional modification of showNew could be clearer using an early return pattern in a separate effect or useMemo.
const showNewLabel = useMemo(() => {
  return showNew && !showBeta;
}, [showNew, showBeta]);
45-45: Extract common styles to constants

The margin adjustments and width styles could be extracted to constants or theme variables for consistency.
const styles = {
  container: {
    width: "100%",
  },
  label: {
    marginLeft: "5px",
  },
} as const;

// Usage in sx prop
sx={{
  ...styles.container,
  cursor: "pointer",
  // ...
}}

// Usage in style prop
style={{
  ...styles.label,
  color: theme.custom.primarySoft,
  fontSize: "11px",
}}
Also applies to: 64-64, 75-75
app/packages/looker/src/elements/image.ts (1)
48-49: Consider extracting the retry delay base as a constant

The magic number 1000 (milliseconds) in the timeout calculation could be extracted as a named constant for better maintainability.
+const BASE_RETRY_DELAY_MS = 1000;
 // ...
-          }, 1000 * this.retryCount);
+          }, BASE_RETRY_DELAY_MS * this.retryCount);
app/packages/core/src/plugins/SchemaIO/components/ContainerizedComponent.tsx (1)
Line range hint 12-44: Consider memoizing the container component selection

The component logic is sound, but consider memoizing the container selection logic to optimize performance when the schema prop changes frequently.
+ const Container = useMemo(() => 
+   container ? containersByName[container.name] : null,
+   [container?.name]
+ );

- const Container = containersByName[container.name];
app/packages/core/src/plugins/SchemaIO/components/NativeModelEvaluationView/ErrorIcon.tsx (1)
1-33: Add accessibility attributes to the SVG icon

While the implementation is clean, consider enhancing accessibility by adding aria labels and role attributes.
 <SvgIcon 
   width="54" 
   height="61" 
   viewBox="0 0 54 61" 
   fill="none" 
+  aria-label="Error icon"
+  role="img"
   {...props}
 >
.github/workflows/test.yml (1)
Line range hint 90-94: Consider adding fail-fast option to codecov upload

The codecov-action upgrade to v5 is good, but consider adding fail-fast option to prevent workflow failures due to coverage upload issues.
 uses: codecov/codecov-action@v5
 with:
   token: ${{ secrets.CODECOV_TOKEN }}
   files: ./coverage.xml
   flags: python
+  fail_ci_if_error: false
.github/workflows/pr.yml (1)
15-15: Fix trailing whitespace

There are trailing spaces that should be removed to maintain consistent formatting.
-  
+
-          (needs.test.result == 'success' || needs.test.result == 'skipped') && 
+          (needs.test.result == 'success' || needs.test.result == 'skipped') &&
Also applies to: 93-93

🧰 Tools

🪛 yamllint (1.35.1)

[error] 15-15: trailing spaces

(trailing-spaces)
app/packages/looker/src/worker/decorated-fetch.test.ts (1)

46-46: Use regex literal instead of RegExp constructor

Replace new RegExp("Max retries for fetch reached") with /Max retries for fetch reached/ for better readability and to avoid unnecessary string escaping.

🧰 Tools

🪛 Biome (1.9.4)

[error] 46-46: Use a regular expression literal instead of the RegExp constructor.

Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.

(lint/complexity/useRegexLiterals)
e2e-pw/src/oss/specs/overlays/detection-mask.spec.ts (1)
Line range hint 54-90: Comprehensive test coverage for mask scenarios

The test cases effectively cover three important scenarios:

Bad detection mask (empty mask)

Good detection mask (filled mask)

Good detection mask path (mask on disk)

Consider adding assertions for each mask type's specific characteristics:
 test("should load all masks fine", async ({ grid, modal }) => {
   await grid.assert.isEntryCountTextEqualTo("3 samples");
+  // Verify bad mask
   await grid.openFirstSample();
   await modal.waitForSampleLoadDomAttribute();
+  await expect(modal.getDetectionMask()).toBeFalsy();
   await modal.close();
+
+  // Verify good mask
+  await grid.openSampleAtIndex(1);
+  await modal.waitForSampleLoadDomAttribute();
+  await expect(modal.getDetectionMask()).toBeTruthy();
+  await modal.close();
+
+  // Verify mask path
+  await grid.openSampleAtIndex(2);
+  await modal.waitForSampleLoadDomAttribute();
+  await expect(modal.getDetectionMask()).toBeTruthy();
+  await modal.close();

   await expect(grid.getForwardSection()).toHaveScreenshot(
app/packages/looker/src/worker/disk-overlay-decoder.test.ts (1)

35-204: Comprehensive test coverage with edge cases!

The test suite thoroughly covers:

Early returns for existing overlays

Successful fetch and decode scenarios

HEATMAP class handling

Recursive DETECTIONS processing

Error handling for failed fetches

However, consider adding tests for:

Concurrent decoding of multiple overlays

Memory usage patterns during large overlay processing
fiftyone/server/metadata.py (1)
442-445: LGTM: Improved path retrieval

Replaced custom deep get implementation with pydash's get function, which is more robust and well-tested.

Consider adding error handling for the path retrieval:
-        path = get(sample, field)
+        try:
+            path = get(sample, field)
+        except Exception as e:
+            logger.debug(f"Failed to get path for field {field}: {e}")
+            continue
fiftyone/utils/torch.py (1)
576-578: LGTM! Consider adding a docstring.

The null check before calling __exit__ on _no_grad is a good defensive programming practice. Consider adding a docstring explaining why this check is necessary.
 def __exit__(self, *args):
+    # Check if _no_grad context manager was initialized before attempting to exit
     if self._no_grad is not None:
         self._no_grad.__exit__(*args)
         self._no_grad = None
fiftyone/utils/data/importers.py (1)
2155-2171: Refactor suggestion: Combine nested if statements

The nested if statements can be combined using and for better readability.
-        if value is None:
-            continue
-
-        if isinstance(value, dict):
+        if value is not None and isinstance(value, dict):
🧰 Tools

🪛 Ruff (0.8.2)

2168-2169: Use a single if statement instead of nested if statements

Combine if statements using and

(SIM102)
docs/source/user_guide/using_datasets.rst (1)
2542-2561: LGTM! Consider clarifying the mask value ranges.

The implementation provides good flexibility by supporting both in-database and on-disk storage of masks. The documentation is clear, but could be enhanced by explicitly stating the valid range for non-zero pixel values in on-disk masks.

Consider adding:
-value indicates the object.
+value (1-255) indicates the object.
fiftyone/utils/cvat.py (1)

Line range hint 4-7: Add tests and documentation

The function needs unit tests and documentation to explain its purpose and parameters.

Would you like me to:

Generate unit tests for this function?

Add docstring documentation?

Create a GitHub issue to track these tasks?
fiftyone/core/collections.py (1)
Line range hint 10665-10711: Improved media field filtering with blacklist support

The changes enhance the flexibility of media field filtering by:

Adding a blacklist parameter to exclude specific fields

Simplifying filepath handling by making it always included

Properly handling app media fields

The blacklist handling code can be simplified using a ternary operator:
- if etau.is_container(blacklist):
-     blacklist = set(blacklist)
- else:
-     blacklist = {blacklist}
+ blacklist = set(blacklist) if etau.is_container(blacklist) else {blacklist}
🧰 Tools

🪛 Ruff (0.8.2)

10702-10705: Use ternary operator blacklist = set(blacklist) if etau.is_container(blacklist) else {blacklist} instead of if-else-block

Replace if-else-block with blacklist = set(blacklist) if etau.is_container(blacklist) else {blacklist}

(SIM108)

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a035528 and e5cd1bd.

⛔ Files ignored due to path filters (6)

app/package.json is excluded by !**/*.json
app/packages/looker/package.json is excluded by !**/*.json
app/yarn.lock is excluded by !**/yarn.lock, !**/*.lock, !**/*.lock
e2e-pw/src/oss/specs/overlays/detection-mask.spec.ts-snapshots/grid-detections-chromium-darwin.png is excluded by !**/*.png, !**/*.png
e2e-pw/src/oss/specs/overlays/detection-mask.spec.ts-snapshots/grid-detections-chromium-linux.png is excluded by !**/*.png, !**/*.png
e2e-pw/yarn.lock is excluded by !**/yarn.lock, !**/*.lock, !**/*.lock

📒 Files selected for processing (75)

.github/workflows/build-docs.yml (1 hunks)
.github/workflows/e2e.yml (1 hunks)
.github/workflows/pr.yml (3 hunks)
.github/workflows/test.yml (1 hunks)
app/packages/app/src/routing/RouterContext.ts (1 hunks)
app/packages/core/src/components/Grid/Grid.tsx (1 hunks)
app/packages/core/src/components/ImageContainerHeader.tsx (1 hunks)
app/packages/core/src/plugins/SchemaIO/components/ContainerizedComponent.tsx (1 hunks)
app/packages/core/src/plugins/SchemaIO/components/NativeModelEvaluationView/Error.tsx (1 hunks)
app/packages/core/src/plugins/SchemaIO/components/NativeModelEvaluationView/ErrorIcon.tsx (1 hunks)
app/packages/core/src/plugins/SchemaIO/components/NativeModelEvaluationView/Evaluate.tsx (1 hunks)
app/packages/core/src/plugins/SchemaIO/components/NativeModelEvaluationView/Evaluation.tsx (33 hunks)
app/packages/core/src/plugins/SchemaIO/components/NativeModelEvaluationView/Overview.tsx (1 hunks)
app/packages/core/src/plugins/SchemaIO/components/NativeModelEvaluationView/Status.tsx (3 hunks)
app/packages/core/src/plugins/SchemaIO/components/NativeModelEvaluationView/utils.ts (1 hunks)
app/packages/looker/src/elements/common/error.ts (1 hunks)
app/packages/looker/src/elements/image.ts (2 hunks)
app/packages/looker/src/elements/video.ts (1 hunks)
app/packages/looker/src/lookers/abstract.ts (8 hunks)
app/packages/looker/src/lookers/frame-reader.ts (1 hunks)
app/packages/looker/src/lookers/utils.test.ts (1 hunks)
app/packages/looker/src/lookers/utils.ts (1 hunks)
app/packages/looker/src/lookers/video.ts (4 hunks)
app/packages/looker/src/numpy.ts (2 hunks)
app/packages/looker/src/overlays/base.ts (4 hunks)
app/packages/looker/src/overlays/detection.ts (3 hunks)
app/packages/looker/src/overlays/heatmap.ts (4 hunks)
app/packages/looker/src/overlays/segmentation.ts (6 hunks)
app/packages/looker/src/worker/canvas-decoder.ts (1 hunks)
app/packages/looker/src/worker/decorated-fetch.test.ts (1 hunks)
app/packages/looker/src/worker/decorated-fetch.ts (1 hunks)
app/packages/looker/src/worker/deserializer.ts (0 hunks)
app/packages/looker/src/worker/disk-overlay-decoder.test.ts (1 hunks)
app/packages/looker/src/worker/disk-overlay-decoder.ts (1 hunks)
app/packages/looker/src/worker/index.ts (11 hunks)
app/packages/looker/src/worker/painter.ts (2 hunks)
app/packages/looker/src/worker/pooled-fetch.test.ts (1 hunks)
app/packages/looker/src/worker/pooled-fetch.ts (1 hunks)
app/packages/looker/src/worker/shared.ts (2 hunks)
app/packages/operators/src/state.ts (3 hunks)
app/packages/plugins/src/index.ts (1 hunks)
app/packages/spaces/src/components/AddPanelButton.tsx (2 hunks)
app/packages/spaces/src/components/AddPanelItem.tsx (4 hunks)
app/packages/state/src/recoil/atoms.ts (1 hunks)
app/packages/state/src/recoil/filters.ts (2 hunks)
app/packages/state/src/recoil/sidebar.test.ts (2 hunks)
app/packages/state/src/recoil/sidebar.ts (3 hunks)
app/packages/utilities/src/fetch.ts (2 hunks)
docs/source/teams/data_lens.rst (4 hunks)
docs/source/user_guide/using_datasets.rst (5 hunks)
e2e-pw/src/oss/specs/grid-tagging.spec.ts (1 hunks)
e2e-pw/src/oss/specs/overlays/detection-mask.spec.ts (4 hunks)
e2e-pw/src/oss/specs/plugins/histograms.spec.ts (1 hunks)
fiftyone/__public__.py (1 hunks)
fiftyone/constants.py (1 hunks)
fiftyone/core/collections.py (4 hunks)
fiftyone/core/dataset.py (2 hunks)
fiftyone/core/labels.py (6 hunks)
fiftyone/core/odm/dataset.py (1 hunks)
fiftyone/core/stages.py (1 hunks)
fiftyone/operators/builtins/panels/model_evaluation/__init__.py (8 hunks)
fiftyone/operators/types.py (4 hunks)
fiftyone/server/metadata.py (8 hunks)
fiftyone/utils/coco.py (2 hunks)
fiftyone/utils/cvat.py (1 hunks)
fiftyone/utils/data/exporters.py (5 hunks)
fiftyone/utils/data/importers.py (2 hunks)
fiftyone/utils/eta.py (1 hunks)
fiftyone/utils/labels.py (10 hunks)
fiftyone/utils/torch.py (1 hunks)
fiftyone/utils/ultralytics.py (1 hunks)
requirements/common.txt (1 hunks)
setup.py (3 hunks)
tests/unittests/import_export_tests.py (1 hunks)
tests/unittests/operators/tableview_tests.py (1 hunks)

💤 Files with no reviewable changes (1)

app/packages/looker/src/worker/deserializer.ts

✅ Files skipped from review due to trivial changes (3)

app/packages/core/src/components/ImageContainerHeader.tsx
app/packages/spaces/src/components/AddPanelButton.tsx
fiftyone/core/stages.py

🧰 Additional context used

📓 Path-based instructions (44)

app/packages/looker/src/lookers/utils.ts (1)