Skip to content

feat(detection): add transform method to remap and filter detections #1846

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

AshAnand34
Copy link

Description

This pull request introduces a new transform method to the Detections class in supervision/detection/core.py, allowing remapping and filtering of detections to match a target dataset's class set. It also includes corresponding unit tests to validate the functionality.

New transform method in Detections:

  • Added the transform method to the Detections class, enabling remapping of class names using an optional class_mapping dictionary and filtering detections to match the classes in a target dataset. The method raises an error if the required class_name field is missing from the .data attribute.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How has this change been tested, please provide a testcase or example of how you tested the change?

Unit tests for transform (in test_transform.py)

  • Added test_transform_remap_and_filter to verify that the method correctly remaps class names, filters out invalid detections, and updates class_id to match the target dataset's class indices.
  • Added test_transform_no_class_mapping to ensure the method works correctly when no class_mapping is provided, retaining only the classes present in the target dataset.
  • Added test_transform_raises_without_class_name to confirm that the method raises a ValueError if the class_name field is missing in the .data attribute.

Copy link
Contributor

@soumik12345 soumik12345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @AshAnand34, thanks for the PR!
I left some comments, it would be great if you address them.

@@ -1435,6 +1435,55 @@ def with_nmm(

return Detections.merge(result)

def transform(self, dataset, class_mapping: Optional[dict] = None) -> Detections:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remap_class_indices would be a more appropriate name for this function.

[class_mapping.get(name, name) for name in class_names]
)
# Filter out detections whose class is not in dataset.classes
keep = np.isin(class_names, dataset.classes)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its better to remove the filtering functionality from the scope of this PR since the filter can be easily applied on the remapped sv.Detections object. Supervision should consist of atomic functionalities that can be chained together as per the developer's requirement.

from supervision.detection.core import Detections


def test_transform_remap_and_filter():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This testcase is failing, please make sure that tests are passing.

@AshAnand34
Copy link
Author

Hi @AshAnand34, thanks for the PR!
I left some comments, it would be great if you address them.

I am out of town at the moment. I will address them when I get back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants