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

Fix issue with missing arguments for centroid_batch function when using asso_func='centroid' #1467

Closed
wants to merge 2 commits into from

Conversation

minicom365
Copy link

The centroid_batch function requires additional w and h arguments, unlike other IOU functions. However, in the current method, even when using the centroid method, only bboxes1 and bboxes2 are passed, resulting in an error.

#1372
Error message: TypeError: centroid_batch() missing 2 required positional arguments: 'w' and 'h'

To resolve this issue, I have modified the method to correctly pass the w and h arguments to the centroid_batch function when the centroid method is used.

Changes made:

Added w and h arguments when calling the centroid_batch function.
This fix will resolve the error when using the centroid method.

Thank you.

@minicom365 minicom365 closed this Jun 9, 2024
@mikel-brostrom
Copy link
Owner

Why was this closed? It makes sense 😄

@minicom365
Copy link
Author

Sorry. This is my first time making a pull request, so I thought "approved these changes" meant it was completed.

@minicom365 minicom365 reopened this Jun 10, 2024
@brianlow
Copy link

Thanks for this change. It fixed the problem for me

@brianlow
Copy link

I wonder if there is a fix that doesn't need an if statement. Seems like the caller shouldn't know about the particular type

I'm just a new user of this package though and am not familiar with the code

@minicom365
Copy link
Author

minicom365 commented Jun 10, 2024

The asso_func passed as an argument is processed in the init method with self.asso_func = get_asso_func(asso_func) and then disappears as a local variable.

To implement it without using an if statement, the original asso_func should be stored within the class in the init method.

Alternatively, you can make the number of arguments for functions other than centroid_batch the same as those for centroid_batch and ignore the extra arguments.

@mikel-brostrom
Copy link
Owner

mikel-brostrom commented Jun 11, 2024

I believe the ultimate solution for his would be:

  1. Extract w, h under update of each tracker

h, w = img.shape[0:2]

  1. All tracker can then use run_asso_func function instead of asso_func:

run_asso_func(asso_func, detections, trackers, w, h)

This is because run_asso_func takes an arbitrary amount of arguments, adapting to the different association functions, something that asso_func isn't capable of.

Are you interested in implementing this @minicom365 , @brianlow?

Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the Stale label Jun 22, 2024
@github-actions github-actions bot closed this Jun 26, 2024
@github-actions github-actions bot removed the Stale label Jun 27, 2024
Copy link

github-actions bot commented Jul 7, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the Stale label Jul 7, 2024
@github-actions github-actions bot closed this Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants