Add mask_path to fo.Detection labels

[Laurent2916]

What changes are proposed in this pull request?

Fix #4486

How is this patch tested? If it is not, please explain why.

I genuinely couldn't figure out how to run the tests, I followed the instructions but kept getting errors when using pytest, so I don't know if I have inadvertently broken something.
I wrote a quick snippet to test the feature instead:

import tempfile
import numpy as np
from PIL import Image
import fiftyone as fo

MASK_SIZE = 256
BBOX = [0, 0, 1, 1]

# create a "mask"
mask = (np.random.rand(MASK_SIZE, MASK_SIZE) * 255).astype(np.uint8)
mask_image = Image.fromarray(mask)
mask_path = tempfile.mktemp(".png")
mask_image.save(mask_path)
print(f"Saved mask to {mask_path}")

# check mask are the same, even though they are stored differently
detection1 = fo.Detection(label="detection1", mask=mask, bounding_box=BBOX)
detection2 = fo.Detection(label="detection2", mask_path=mask_path, bounding_box=BBOX)
assert np.allclose(detection1.get_mask(), detection2.get_mask()), "detection masks are different"  # type: ignore
print(detection1, detection2)

# check polyline conversion
polyline1 = detection1.to_polyline()
polyline2 = detection2.to_polyline()
assert np.allclose(polyline1.points, polyline2.points), "polyline points are different" # type: ignore

# check mask export
mask2_path = tempfile.mktemp(".png")
detection1.export_mask(mask2_path, update=True)
print(f"Exported mask to {mask2_path}")
mask2_image = Image.open(mask2_path)
mask2 = np.array(mask2_image)
assert np.allclose(mask, mask2), "exported mask is different"
print(detection1, detection2)

# check mask import
detection2.import_mask(update=True)
assert np.allclose(detection1.get_mask(), detection2.get_mask()), "imported mask is different"  # type: ignore
print(detection1, detection2)

Release Notes

Is this a user-facing change that should be mentioned in the release notes?

• No. You can skip the rest of this section.
• Yes. Give a description of this change to be included in the release notes for FiftyOne users.

Allows Detections to store masks on disk, this allows to offload some disk usage from the mongo database to somewhere else. See also #4486

What areas of FiftyOne does this PR affect?

• App: FiftyOne application changes
• Build: Build and test infrastructure changes
• Core: Core fiftyone Python library changes
• Documentation: FiftyOne documentation changes
• Other

Summary by CodeRabbit

• New Features

  • Introduced new capabilities for managing instance segmentation masks in the Detection class, including import/export functionality and transformations.
  • Enhanced mask retrieval processes across various modules for improved maintainability.

• Bug Fixes

  • Updated checks for mask presence to utilize the new mask retrieval methods, ensuring consistent access and functionality.

[coderabbitai]

Walkthrough

The recent updates to the Detection class in FiftyOne enhance the handling of instance segmentation masks by introducing a mask_path attribute and several methods for managing masks. These changes allow users to efficiently manage mask data stored on disk, promoting better encapsulation and maintainability.

Changes

Files	Change Summary
fiftyone/core/labels.py	Introduced mask_path for on-disk segmentation masks; added methods for mask management: has_mask, get_mask, import_mask, export_mask, and updated to_segmentation.
fiftyone/utils/coco.py	Switched mask access from direct attribute to has_mask() method in from_label and _coco_objects_to_detections, enhancing encapsulation.
fiftyone/utils/cvat.py	Updated mask presence check in _create_detection_shapes to use has_mask(), improving consistency and maintainability.
fiftyone/utils/eta.py	Changed mask retrieval in to_detected_object from direct access to get_mask(), indicating possible additional processing or validation.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Detection
    participant Disk

    User->>Detection: Create Detection with mask_path
    Detection->>Disk: Import mask from mask_path
    Detection->>User: Mask available for use
    User->>Detection: Request mask
    Detection->>Detection: Get mask via get_mask()
    Detection->>User: Provide mask

Loading

Assessment against linked issues

Objective	Addressed	Explanation
Allow mask_path in Detection labels (#4486)	✅
Enable offloading masks from the mongo database (#4486)	✅

🐇 "In the fields where the data flows,
A new path for masks now brightly glows.
From disk they come, so light and free,
Encapsulated magic, just wait and see!
With joyful hops, we celebrate this change,
In FiftyOne's world, we’ll grow and rearrange!" 🌼

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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>.
  • 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 generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @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 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.

Early access features: disabled

We are currently testing the following features in early access:

• Anthropic claude-3-5-sonnet for code reviews: Anthropic claims that the new Claude model has stronger code understanding and code generation capabilities than their previous models. Note: Our default code review model was also updated late last week. Please compare the quality of the reviews between the two models by toggling the early access feature.

Note:

• You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.
• Please join our Discord Community to provide feedback and report issues on the discussion post.

[coderabbitai]

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 991c1d0 and f13a8c5.

Files selected for processing (4)

• fiftyone/core/labels.py (2 hunks)
• fiftyone/utils/coco.py (2 hunks)
• fiftyone/utils/cvat.py (1 hunks)
• fiftyone/utils/eta.py (1 hunks)

Additional comments not posted (12)

fiftyone/utils/eta.py (1)

599-599: Use of get_mask() enhances encapsulation.

The change from accessing detection.mask directly to using detection.get_mask() improves encapsulation and potentially adds validation or processing. Ensure that get_mask() is correctly implemented to handle all necessary cases.

fiftyone/core/labels.py (8)

404-405: Introduction of mask_path enhances flexibility.

The addition of mask_path allows masks to be stored on disk, reducing database load. Ensure that paths are handled securely to prevent path traversal vulnerabilities.

---

412-413: Declaration of _MEDIA_FIELD improves media handling.

Setting _MEDIA_FIELD to "mask_path" indicates a shift towards handling media files, aligning with the new mask_path feature.

---

421-424: has_mask property improves mask presence checks.

This property efficiently checks for the presence of a mask, whether in memory or on disk. Ensure it is used consistently throughout the class.

---

426-438: get_mask method centralizes mask retrieval.

Using get_mask to access the mask encapsulates the logic for retrieving masks from memory or disk. Verify that _read_mask handles file reading securely and efficiently.

---

440-454: import_mask method facilitates mask importation.

This method allows importing masks from disk into memory, with an option to clear mask_path. Ensure _read_mask is robust against file-related errors and that mask_path is cleared only when intended.

---

455-472: export_mask method supports mask exportation.

The method exports masks to disk, updating attributes as needed. Ensure that the file operations handle errors gracefully and that path updates are consistent.

---

473-514: transform_mask method enables mask transformation.

This method provides flexibility in transforming masks, with options to save changes. Ensure that _transform_mask handles transformations correctly and that file operations are secure.

---

570-571: to_segmentation method utilizes get_mask.

Updating this method to use get_mask ensures consistent mask retrieval. Ensure that error handling is robust for cases where masks are not available.

fiftyone/utils/coco.py (2)

1302-1302: Good use of encapsulation with get_mask().

The change to use label.get_mask() instead of direct attribute access improves encapsulation and maintainability. Ensure that the get_mask method is correctly implemented to handle all necessary logic for mask retrieval.

---

2113-2113: Improved maintainability with get_mask().

Using detection.get_mask() instead of direct attribute access centralizes mask retrieval logic, which is beneficial for maintainability. Verify that the get_mask method is implemented to handle all relevant cases.

fiftyone/utils/cvat.py (1)

6403-6403: Good encapsulation practice with get_mask().

The use of det.get_mask() instead of directly accessing det.mask improves encapsulation and maintainability by potentially incorporating additional logic within the getter method.

[brimoor]

Hi @Laurent2916! 👋

Looks like you're interested in working with detection tasks where the object masks are stored as images on disk rather than as numpy arrays in the database.

Out of curiosity, how big are your instance masks in your use cases? In terms of resolution, number of objects per image, etc.

From a data modeling precedent standpoint, we have another closely related proposal in #4500. In that case, the objective is also to work with detection datasets defined by on-disk images rather than in-database arrays, but there is an additional assumption that the objects are strictly non-overlapping and thus it is proposed to be achieved by adding a single full-resolution mask at the Detections.mask|mask_path level.

There's a quite detailed discussion there about how in the author's opinion, object detections represented in that way should be distinct from the existing Segmentation label type, which is identical from a data storage standpoint (single per-image mask) but has different semantics in terms of the pixel values representing instances rather than classes.

I'm pointing out #4500 because, while we could in principle have all three of these:

• Segmentation.mask_path
• Detections.mask_path
• Detection.mask_path

the added representational flexibility incurs extra complexity in making the rest of fiftyone work (in-App visualization, import/export, additional complexities to support cloud-backed datasets in FiftyOne Teams, etc).

[brimoor]

Note to Voxel51, this would be the first instance of a media field that could be embedded in a list (Detections), which means these methods will need to be updated:

• fiftyone/fiftyone/utils/data/exporters.py

  Line 2345 in 1824ddb

  def _export_media_field(self, d, field_name, key=None):

• fiftyone/fiftyone/utils/data/exporters.py

  Line 2041 in 1824ddb

  def _export_media_field(self, d, field_name, key=None):

There could be more such places too.

[coderabbitai]

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f13a8c5 and b0147a0.

Files selected for processing (3)

• fiftyone/core/labels.py (2 hunks)
• fiftyone/utils/coco.py (2 hunks)
• fiftyone/utils/cvat.py (1 hunks)

Additional comments not posted (8)

fiftyone/core/labels.py (5)

421-424: LGTM!

The code changes are approved.

---

426-438: LGTM!

The code changes are approved.

---

440-453: LGTM!

The code changes are approved.

---

455-471: LGTM!

The code changes are approved.

---

527-528: LGTM!

The code changes are approved.

fiftyone/utils/coco.py (2)

1302-1302: Addressed previous comment.

The code has been updated to use label.has_mask() as suggested in the previous comment for better performance.

---

2113-2113: LGTM!

The code changes are approved. The check ensures that the detection is included only if segmentations are not being loaded or if the detection has a mask.

fiftyone/utils/cvat.py (1)

6403-6403: Use has_mask() method to check for detection masks.

Good update to use the has_mask() method here instead of directly checking the mask attribute. This encapsulates the logic for determining mask existence.

[Laurent2916]

Hi @brimoor!

Thanks for the suggestions, I've applied them.

I'm working with ~1000x1000px masks on average. I have about 30k images for now, with each image containing 2-4 masks. This means that currently, masks are responsible for a good chunk of the disk usage of my mongo db.

The masks I have aren't "non-overlapping", they are inferred from various segmentation models and thus overlap most of the time (they are also non-binary/grayscale, even though the UI doesn't support displaying this). So using Detections.mask_path doesn't quite fit my use-case I'm afraid.

[Laurent2916]

Hi @brimoor, when you get a chance, could you please take a look again at this PR and let me know your thoughts ?

[sashankaryal]

Hi @Laurent2916, we'll continue this in #5120