Skip to content
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

[DAR-3771][External] Import of annotation-level properties #925

Merged
merged 5 commits into from
Sep 16, 2024
Merged

Conversation

JBWilkie
Copy link
Collaborator

@JBWilkie JBWilkie commented Sep 12, 2024

Problem

darwin-py is currently incapable of handling:

  • 1: A per-property granularity field in the .v7/metadata.json properties manifest file
  • 2: frame_index=null values for annotation-level property values

Both concepts must be supported by darwin-py to allow the same import functionality as section-level properties (which are currently supported properties)

Solution

  • Extend the Property class with granularity
  • Introduce the PropertyGranularity enum to represent the 3 property granularities

Adjusted the properties import functions to ensure :

  1. Annotation-level property values can be imported, but that granularity is optional so that section-level property values can still be imported
  2. Annotation-level property values present in the properties manifest but that are missing in the team can be created in the team
  3. Annotation-level properties present in the properties manifest but that are missing in the team can be created in the team

We also add unit tests for _import_annotations, as none previously existed. These test that for both section & annotation-level properties:

  • Existing property values can be imported as expected
  • New property values can be created from the manifest file
  • New properties themselves can be created from a manifest file, and section-level properties can still be created without specifying a granularity

Changelog

Added support for annotation-level properties

@@ -38,6 +42,7 @@ def base_property_object(base_property_value: PropertyValue) -> FullProperty:
annotation_class_id=0,
property_values=[base_property_value],
options=[base_property_value],
granularity=PropertyGranularity("section"),

Choose a reason for hiding this comment

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

This is questionable: currently the BE has a default 'section' if no granularity is provided. Here we're building a fixture that disregards the BE default. It's not wrong, but I think it'd be interesting to also have a test that doesn't specify the granularity at all when creating a property

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The unit tests I just added test creating section-level properties from a manifest file where no granularity is specified

@JBWilkie JBWilkie force-pushed the DAR-3771 branch 2 times, most recently from 93d1f4b to 0cba8ac Compare September 13, 2024 14:25
Comment on lines 370 to +375
): # using numel() to check if tensor is non-empty
print("WE GOT MASKS")
albumentation_dict["masks"] = masks.numpy()
if isinstance(masks, torch.Tensor):
masks = masks.numpy()
if masks.ndim == 3: # Ensure masks is a list of numpy arrays
masks = [masks[i] for i in range(masks.shape[0])]
albumentation_dict["masks"] = masks
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Change that is unrelated to the ticket, but necessary to resolve these failing tests:

@JBWilkie JBWilkie merged commit 9f4530d into master Sep 16, 2024
24 checks passed
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.

3 participants