Skip to content

Conversation

@kkim-labelbox
Copy link
Contributor

@kkim-labelbox kkim-labelbox commented May 18, 2022

Includes two changes:
AL-2496: Rename custom_metadata to metadata_fields
AL-2498: allows create_data_row to take in a dictionary, as well as kwargs

Related API change - https://github.com/Labelbox/intelligence/pull/8623

@kkim-labelbox kkim-labelbox changed the title [AL-2496] Rename custom_metadata to metadata_fields for DataRow [AL-2496] [AL-2498] Rename custom_metadata to metadata_fields for DataRow May 18, 2022
Copy link

@nmaswood nmaswood left a comment

Choose a reason for hiding this comment

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

LGTM

In the future I would recommend breaking things like:

  • renaming
  • adding fields items

into separate PRs because we can discuss and review them as separate line items

"""
invalid_argument_error = "Argument to create_data_row() must be either a dictionary, or kwargs containing `row_data` at minimum"

def convert_field_keys(items):

Choose a reason for hiding this comment

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

I maybe move this out of the function scope and into module scope because this seems like a more general utility function type thing and doesn't need to be re-defined every time this method is called


DataRow = Entity.DataRow
if DataRow.row_data.name not in kwargs:
args = convert_field_keys(items) if items is not None else kwargs

Choose a reason for hiding this comment

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

if if items is a little more concise

for key, value in items.items()
}

if items is not None and len(kwargs) > 0:

Choose a reason for hiding this comment

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

if items is maybe more concise and strict

@kkim-labelbox kkim-labelbox merged commit 8389ca2 into develop May 21, 2022
@jtsodapop jtsodapop deleted the kkim/AL-2496 branch September 14, 2022 15:52
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