Skip to content

Conversation

whistler
Copy link
Contributor

When creating a Label, the annotation data type class was being changed and the global_key field was disappearing. Example:

label = []
label.append(
  lb_types.Label(
    data=lb_types.ConversationData(
      global_key=global_key
    ),
    annotations=[
      ner_annotation
    ]
  )
)

Output

[Label(uid=None, data=VideoData(file_path=None,frames=None,url=None), annotations=[ObjectAnnotation(confidence=None, name='ner', feature_schema_id=None, extra={}, value=ConversationEntity(start=0, end=8, extra={}, message_id='4'), classifications=[])], extra={})]

The problem was that Pydantic tries to coerce objects to the first class listed in a Union. See Pydantic docs for more details.

This is fixed by adding a discriminator field (class_name) which is ignored during serialization.

assert label.annotations[0].confidence == 0.914


def test_initialize_label_no_coersion():
Copy link
Contributor

Choose a reason for hiding this comment

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

small typo here, coersion => coercion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! fixed!

Copy link
Contributor

Choose a reason for hiding this comment

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

awesome work BTW!

class DicomData(BaseData):
...
class DicomData(BaseData, _NoCoercionMixin):
data_type: Literal["DicomData"] = "DicomData"
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be class_name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch! fixed it.

"""
uid: Optional[Cuid] = None
data: Union[VideoData, ImageData, TextData, TiledImageData]
data: DataType
Copy link
Contributor

Choose a reason for hiding this comment

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

is this a generic pydantic type? sorry pydantic documentation links do not work for me for some reason

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've defined this type on line 19 above since the list was getting too long.

Copy link
Contributor

@vbrodsky vbrodsky left a comment

Choose a reason for hiding this comment

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

Fantastic, we should extend it to all our union types... as you know they are all susceptible

@whistler whistler force-pushed the imuhammad/fix-data-type-coersion branch 4 times, most recently from 9ace2a2 to 3e47e5c Compare March 17, 2023 23:15
@whistler whistler force-pushed the imuhammad/fix-data-type-coersion branch from 3e47e5c to a50ca2b Compare March 17, 2023 23:35
@whistler whistler force-pushed the imuhammad/fix-data-type-coersion branch from a50ca2b to 2959247 Compare March 18, 2023 00:14
@whistler whistler force-pushed the imuhammad/fix-data-type-coersion branch from 2959247 to 5ffc6dc Compare March 21, 2023 01:45
@whistler whistler merged commit 817335e into develop Mar 21, 2023
@whistler whistler deleted the imuhammad/fix-data-type-coersion branch March 21, 2023 17:27
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.

5 participants