-
Notifications
You must be signed in to change notification settings - Fork 82
Pull Request: Add SecondSpectrum Event Data Support and updated tracking data deserialiser. #437
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?
Conversation
Update the `secondspectrum.load` function to handle the absence of the 'fps' key in newer files. * Modify `kloppy/infra/serializers/tracking/secondspectrum.py` to check for the presence of the 'fps' key in the metadata and use a default frame rate of 25.0 if the 'fps' key is absent. * Add a test case in `kloppy/tests/test_secondspectrum.py` to verify that the `secondspectrum.load` function handles the absence of the 'fps' key correctly. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/WoutPaepenUcLL/kloppy?shareId=XXXX-XXXX-XXXX-XXXX).
Fix fps key handling in secondspectrum.load function
pitchLength and width are nested in "data"
…format --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/WoutPaepenUcLL/kloppy?shareId=XXXX-XXXX-XXXX-XXXX).
Update secondspectrum load function to support current metadata.json format
metadata nested error fix
…y and deserializer
Add event data loading and deflection event classes
…sive tests for event deserialization
Add-event-data-secondspectrum
|
Hi Wout, Really cool PR! I haven't looked into it in-depth, but a couple quick things:
|
Event Support Status in SecondSpectrum DeserializerHere is a table with all events and if it is supported by the deserialiser or not:
|
|
@WoutPaepenUcLL it looks like you have to run Also, you did not include "receival" in the above table, is that correct? |
|
i did not see the "receival" type in the event_types. But it is not yet implemented. I think i implemented it wrong by looking if a pass is complete or not, so it just skips the event in the deserialiser when it is a "receival". |
|
Ah, no that makes sense. Apologies for the confusion! I noticed the |
|
Are there anymore things i need to change? |
|
@WoutPaepenUcLL I'm trying to source some SecondSpectrum event data so that I can further and more accurately review the PR. Hopefully I'll have an update soon. |
|
Hi @WoutPaepenUcLL thanks for your patience, I've finally sourced some SecondSpectrum data. Here is a list of feedback. I'm mostly following our new (yet to be released) contribution guide on Adding a New Event Data Provider. Loading
def load_event(
event_data: FileLike,
meta_data: FileLike,
event_types: Optional[List[str]] = None,
coordinates: Optional[str] = None,
event_factory: Optional[EventFactory] = None,
) -> EventDataset:
from ._providers.secondspectrum import load, load_event, load_tracking
__all__ = ["load", "load_event", "load_tracking"]Parsing
with performance_logging("parse events", logger=logger):
parsed_events = []
items_list = list(raw_events.items())
for i, (event_id, raw_event) in enumerate(iter(raw_events.items())):
if raw_event["type"] == "reception":
continue
if raw_event["type"] == "pass":
if i + 1 < len(items_list):
next_id, next_event = items_list[i + 1]
if next_event["type"] == "reception":
reception_event = next_event
else:
reception_event = NoneThen, we could pass reception_event into
position_mapping.get(
player_data["position"],
PositionType.Unknown,
)
General
DocumentationEventhough the docs are not updated yet, we should probably attempt to update event-spec.yml to align with the table you created above, after you've updated the PR. In this yml file we differentiate between "parsed", "not supported" (by the data provider), "not implemented" (in kloppy), "unknown" and "inferred". |
Pull Request: Add SecondSpectrum Event Data Support
Description
This PR enhances kloppy to support loading and processing event data from SecondSpectrum. It adds new event types, improves the metadata handling in the deserializer, and includes comprehensive tests for event deserialization.
These changes are made with documentation provided by secondspectrum.
Changes
SecondSpectrumDeserializerDeflectionEventandDeflectionResultclassesImplementation Details
secondspectrum.pyto handle the current metadata.json formatsecondspectrum_fake_eventdata.jsonl) for testingTesting
Added comprehensive tests to verify:
Future Work
This PR builds upon several smaller fixes and enhancements that were previously merged, consolidating them into a comprehensive solution for SecondSpectrum event data handling.