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

tracker_id contents are never None #1214

Merged
merged 2 commits into from
May 24, 2024

Conversation

LinasKo
Copy link
Collaborator

@LinasKo LinasKo commented May 21, 2024

Description

I've verified every instance of tracker_id in our repo, and found that its contents are never None.
The checks we have are not necessary.

In cases where tracker_id gets -1 added, it is indeed filtered out very soon.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

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

Basic tests of smoother and line counter can be found here: https://colab.research.google.com/drive/1_gajR0D_CZ4Z2dZ1y8SdVaWGbDZ0mBpl?usp=sharing

Even with no detections for longer periods of time, no errors (and no None values) are produced.

Any specific deployment considerations

None

Docs

  • Docs updated? What were the changes:

@LinasKo LinasKo linked an issue May 21, 2024 that may be closed by this pull request
2 tasks
@onuralpszr
Copy link
Collaborator

fyi, @LinasKo collab access is not public

@LinasKo
Copy link
Collaborator Author

LinasKo commented May 22, 2024

@onuralpszr, thank you - I've opened it up

@onuralpszr
Copy link
Collaborator

@LinasKo how about make another rc release with it ? For better tracker fix ?

@LinasKo
Copy link
Collaborator Author

LinasKo commented May 22, 2024

@LinasKo how about make another rc release with it ? For better tracker fix ?

That's outside my area of responsibility 🙂

Our main goal, however, is working towards the new release, rather than pushing out more candidate packages.

@SkalskiP
Copy link
Collaborator

Hi @onuralpszr 👋🏻 why do you think we should release a new rc?

@onuralpszr
Copy link
Collaborator

Hi @onuralpszr 👋🏻 why do you think we should release a new rc?

Well, current release has tracker issue and with those fix and this extra addition should be live for others to easy to use. Personally, I wish we should do point release instead of major release.

@onuralpszr
Copy link
Collaborator

I can also update current doc version without even change version number If you like (like last time we did)

@SkalskiP
Copy link
Collaborator

SkalskiP commented May 24, 2024

Well, current release has tracker issue and with those fix and this extra addition should be live for others to easy to use.

I understand. I'm also not particularly happy that the latest version has had this bug for so long, but we will have the next release soon – in a week. We still have some inconsistencies, such as #1215. I would prefer to resolve these before the release.

@SkalskiP SkalskiP merged commit f40dd38 into develop May 24, 2024
9 checks passed
@onuralpszr
Copy link
Collaborator

Well, current release has tracker issue and with those fix and this extra addition should be live for others to easy to use.

I understand. I'm also not particularly happy that the latest version has had this bug for so long, but we will have the next release soon – in a week. We still have some inconsistencies, such as #1215. I would prefer to resolve these before the release.

If time is close, We should be fine.

@onuralpszr onuralpszr deleted the fix/remove-none-tracker-id-contents-checks branch September 23, 2024 15:18
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.

sv.Detections has exactly N of each member, except for tracker_id.
3 participants