-
Notifications
You must be signed in to change notification settings - Fork 84
[HAWKEYE] Add object_id to loader
#473
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
base: master
Are you sure you want to change the base?
[HAWKEYE] Add object_id to loader
#473
Conversation
probberechts
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only had a quick look at code, but it's not immediately clear to me how this is supposed to work.
| limit: Optional[int] = None, | ||
| coordinates: Optional[str] = None, | ||
| show_progress: Optional[bool] = False, | ||
| object_id: Optional[str] = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add this to the docstring?
| super().__init__(limit, sample_rate, coordinate_system) | ||
| self.object_id: HawkEyeObjectIdentifier = None | ||
| self.object_id: HawkEyeObjectIdentifier = ( | ||
| None if object_id is None else object_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is identical to
self.object_id = object_idThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also do not see where the user-provided object_id is taken into account.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm running into some issues because I removed this branch, but the goal of the object_id is to allow users to specify an object_id that is not any of those 3 (heId, fifaId or uefaId) in a case when they get their data and it does not have any of these keys to identify player and team objects.
I'll redo this PR at some point.
Currently we only support "fifaId", "uefaId" and "heId" as HawkEye identifiers, because we cannot be sure this is all of them, we allow the user to pass an optional
object_idparameter.