Skip to content

Conversation

@kkim-labelbox
Copy link
Contributor

@kkim-labelbox kkim-labelbox commented Mar 11, 2023

  • Adds global_key to Annotation Types and supports NDJsonConverter serialization and deserialization
  • Added test cases for all currently supported data types
  • Validate that both globalKey and id cannot be supplied in annotation imports (only for MALPredictionImport, MEAPredictionImport and LabelImport)

return v
@root_validator()
def must_set_one(cls, values):
if bool(values.get('id')) == bool(values.get('global_key')):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if bool(values.get('id')) == bool(values.get('global_key')):
if bool(values.get('id')) == bool(values.get('global_key')) == None: # to be on a safe side

Copy link
Contributor

Choose a reason for hiding this comment

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

Since this logic can be a little tricky to read, might want to create a util function that describes what it does, e.g. is_exactly_one_set(values.get('id'), values.get('global_key'))

return v
@root_validator()
def must_set_one(cls, values):
if bool(values.get('id')) == bool(values.get('global_key')):
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this logic can be a little tricky to read, might want to create a util function that describes what it does, e.g. is_exactly_one_set(values.get('id'), values.get('global_key'))

Copy link
Contributor

@whistler whistler left a comment

Choose a reason for hiding this comment

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

LGTM!

@kkim-labelbox kkim-labelbox merged commit 44f843a into develop Mar 14, 2023
@kkim-labelbox kkim-labelbox deleted the kkim/AL-5172 branch March 14, 2023 16:17
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.

4 participants