Skip to content

Update create_coco_tf_record.py#10820

Merged
copybara-service[bot] merged 7 commits intomasterfrom
sineeli-patch-2
Nov 21, 2022
Merged

Update create_coco_tf_record.py#10820
copybara-service[bot] merged 7 commits intomasterfrom
sineeli-patch-2

Conversation

@sineeli
Copy link
Copy Markdown
Contributor

@sineeli sineeli commented Nov 3, 2022

Description

Code breaks when bounding box coordinates crosses the original image width or height itself.

  • When number of bbox_annotations is same as num_annotations_skipped in a single example include a check, not to create the feature_dict and just return any empty feature_dict.
  • Later included another check in tfrecord_lib.py whether tf_example is not empty while writing to output tfrecord file.

Type of change

For a new feature or function, please create an issue first to discuss it
with us before submitting a pull request.

Note: Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

Tests

📝 Please describe the tests that you ran to verify your changes.

  • Provide instructions so we can reproduce.
  • Please also list any relevant details for your test configuration.

Test Configuration:

Checklist

When number of `bbox_annotations` is same as `num_annotations_skipped` in a single example include a check, not to create the `feature_dict` and just return any empty `feature_dict`.
@sineeli sineeli requested review from luotigerlsx and removed request for jaeyounkim, rachellj218 and saberkun November 3, 2022 19:09
@luotigerlsx
Copy link
Copy Markdown
Member

Hey @sineeli, thanks for the PR. I have added two comments.

proto = tfrecord_lib.convert_to_feature([b'123', b'456'])
self.assertSequenceAlmostEqual(proto.bytes_list.value, [b'123', b'456'])

def test_obj_annotation_tf_example(self):
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@luotigerlsx

I have added the test case, can you please check and review the same. Really thanks for reviewing the code.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for adding the test case. It looks good to me. But due to the newly added code, the lint test doesn't pass. Please check the log the modify accordingly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I made all the corrections which are required from lint of point of view. Thanks

@sineeli sineeli added the ready to pull ready to pull (create internal pr review and merge automatically) label Nov 15, 2022
@sineeli sineeli requested a review from saberkun November 15, 2022 18:43
@sineeli sineeli removed the ready to pull ready to pull (create internal pr review and merge automatically) label Nov 15, 2022
@sineeli sineeli added ready to pull ready to pull (create internal pr review and merge automatically) models:official models that come under official repository labels Nov 15, 2022
copybara-service bot pushed a commit that referenced this pull request Nov 21, 2022
@copybara-service copybara-service bot merged commit bbe6e64 into master Nov 21, 2022
@sineeli sineeli deleted the sineeli-patch-2 branch November 21, 2022 17:39
@sineeli sineeli restored the sineeli-patch-2 branch November 21, 2022 17:56
@saberkun saberkun deleted the sineeli-patch-2 branch November 23, 2022 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

models:official models that come under official repository ready to pull ready to pull (create internal pr review and merge automatically)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants