Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding ByteTrack unit tests #1077

Conversation

rolson24
Copy link
Contributor

Description

This PR adds unit tests for ByteTrack. They do not all pass because of a bug introduced in the last PR, but I have tested them
on the current release of supervision and they pass correctly. They also pass after making the 1 line bug fix.
I chose the test cases to be the ones that I have encountered to be edge cases so far, as well as adding edge cases that I can imagine being problematic.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

How has this change been tested, please provide a testcase or example of how you tested the change?

I tested this change, by first running these pytests on the 0.18.0 release version of supervision to ensure they all pass. I then used them to validate my other changes to ByteTrack to ensure the changes still function correctly. I also tested the current develop branch, and 1 test failed due to a small bug in how update_with_detections handles no valid tracked detections. (added here)

Any specific deployment considerations

Need to make sure they tests will autorun on PR.

Docs

  • Docs updated? What were the changes:

Copy link

@Itayflymingo Itayflymingo left a comment

Choose a reason for hiding this comment

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

n

@LinasKo
Copy link
Collaborator

LinasKo commented May 23, 2024

Hi @rolson24 👋

We've been discussing internally on how to test composite tools such as the ByteTracker. In many respects it's a collection of functions that should rest on a thorough foundation of unit tests. On the other hand, it could be seen as more suited for integration tests.

I liked how your tests are implemented, but we'd prefer to lay a broader foundation for ByteTracker, covering many more cases, rather than adding a few fixtures and iterating later.

@LinasKo LinasKo closed this May 23, 2024
@SkalskiP
Copy link
Collaborator

SkalskiP commented May 24, 2024

Hi @rolson24! 👋🏻 Although we decided not to merge this PR, thank you very much for the time and effort you put into the development of supervision.

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.

None yet

4 participants