-
Notifications
You must be signed in to change notification settings - Fork 82
[SPORTEC] Add some missing Sportec events #480
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
Open
mrkthmpsn
wants to merge
16
commits into
PySport:master
Choose a base branch
from
mrkthmpsn:mrkthmpsn/add-missing-sportec-features
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
[SPORTEC] Add some missing Sportec events #480
mrkthmpsn
wants to merge
16
commits into
PySport:master
from
mrkthmpsn:mrkthmpsn/add-missing-sportec-features
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Taken from the IDSSE dataset, to be used when testing updates to the Sportec deserializer
I'm not sure whether this is just the test file or is something systematic with Sportec data, that certain event types are grouped at the end of the file despite their timestamps placing them throughout the match
Based on the data attributes and visualising some examples of events, I think the interpretations in the code comments are correct. I suspect that 'layoff' is a kind of 'loose ball'/'no outcome' variant, but I'm not confident on that interpretation or what it would be classed as in kloppy
…sing-sportec-features
- fix starting position of sub players - fix dataset flags - add formation - add metadata tests
52f09ea to
9bd4f5b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Picking up some of the things highlighted in #442
Of those items:
CardColorandRecoveryare already being processedTacklingGamevariations that I found in open data files. Of those that I identified, I'm only unsure aboutWinnerResult=="layoff"variationsThe PR also adds a logic step to order events by timestamp before iterating over the events list to parse them. In the open data files, I found that some of these events were all at the end of the file, which interfered with assigning match periods (everything got put in the second half).
As mentioned in #442, this PR also adds one of the open event and metadata files as test files. There's a new deserialization test based on these files, as the old/existing test files don't include a lot of them.