Skip to content

fix(sanity): pass if the reference token is empty string#240

Merged
ktro2828 merged 1 commit into
mainfrom
fix/sanity/pass-if-reference-is-empty-string
Dec 3, 2025
Merged

fix(sanity): pass if the reference token is empty string#240
ktro2828 merged 1 commit into
mainfrom
fix/sanity/pass-if-reference-is-empty-string

Conversation

@ktro2828
Copy link
Copy Markdown
Collaborator

@ktro2828 ktro2828 commented Dec 2, 2025

What

This pull request introduces a minor adjustment to the reference check logic in base.py. The change ensures that records with an empty string as the reference token are not flagged as missing, improving the accuracy of the sanity check.

  • Logic Improvement:
    • Updated the check method in t4_devkit/sanity/reference/base.py to skip records where the reference token is an empty string, treating them as successful matches.

Usecase

If the T4 dataset contains only 2D annotations (object_annotation.json), both first_annotation_token and last_annotation_token will be empty string "", although instance records are generated.

Therefore, fist_annotation_token and last_annotation_token should be the reference to sample_annotation.json.

How PR Tested?

  REF010:
     - No reference to 'instance.first_annotation_token':
     - No reference to 'instance.first_annotation_token':
  REF011:
     - No reference to 'instance.last_annotation_token':
     - No reference to 'instance.last_annotation_token':

+--------------------------------------+---------+--------+--------+---------+----------+
|              DatasetID               | Version | Passed | Failed | Skipped | Warnings |
+--------------------------------------+---------+--------+--------+---------+----------+
| 471a3751-dac2-451e-9421-552975d50272 |         |   47   |   2    |    2    |    4     |
+--------------------------------------+---------+--------+--------+---------+----------+
  • After
  REF010: ✅
  REF011: ✅

+--------------------------------------+---------+--------+--------+---------+----------+
|              DatasetID               | Version | Passed | Failed | Skipped | Warnings |
+--------------------------------------+---------+--------+--------+---------+----------+
| 471a3751-dac2-451e-9421-552975d50272 |         |   49   |   0    |    2    |    4     |
+--------------------------------------+---------+--------+--------+---------+----------+

Signed-off-by: ktro2828 <kotaro.uetake@tier4.jp>
Copilot AI review requested due to automatic review settings December 2, 2025 12:46
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the reference validation logic to properly handle empty string reference tokens in T4 datasets that only contain 2D annotations. When a dataset lacks 3D annotations, first_annotation_token and last_annotation_token fields are legitimately empty strings, and should not be flagged as missing references.

Key changes:

  • Modified the reference check in base.py to treat empty string reference tokens as valid, preventing false positive validation failures.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions github-actions Bot added the bug Something isn't working label Dec 2, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 2, 2025

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
3971 3267 82% 50% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
t4_devkit/sanity/reference/base.py 87% 🟢
TOTAL 87% 🟢

updated for commit: a5e74f5 by action🐍

@ktro2828 ktro2828 changed the title fix: pass if the reference token is empty string fix(sanity): pass if the reference token is empty string Dec 2, 2025
@ktro2828
Copy link
Copy Markdown
Collaborator Author

ktro2828 commented Dec 2, 2025

@SamratThapa120 Let me just confirm!
Do you agree with the following idea?

If the T4 dataset contains only 2D annotations (object_annotation.json), both first_annotation_token and last_annotation_token will be empty string "", although instance records are generated.

Therefore, fist_annotation_token and last_annotation_token should be the reference to sample_annotation.json.

As far as I explored, tier4_perception_dataset/AnnotationFileGenerator generate references from an instance record to the first and last annotation records only for 3D annotations, not for 2D.

We can update generator as follows, but it cause conflicts if the dataset contains both 3D and 2D.

Then, I think we should define thafirst_annotation_token and last_annotation_token only refer to sample_annotation records, and they would be empty string "" if the dataset only contains 2D annotations.

Copy link
Copy Markdown
Contributor

@shekharhimanshu shekharhimanshu left a comment

Choose a reason for hiding this comment

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

Thank you for the fix. LGTM! 💯

@ktro2828 ktro2828 merged commit c8ab6fb into main Dec 3, 2025
4 of 5 checks passed
@ktro2828 ktro2828 deleted the fix/sanity/pass-if-reference-is-empty-string branch December 3, 2025 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants