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

Add InterceptionEvent #230

Merged
merged 5 commits into from
Nov 16, 2023

Conversation

MKlaasman
Copy link
Contributor

fixes #222

Notes:

  • Whenever needed (wyscout & opta), I inferred the InterceptionResult from the next event. Though, this approach might still be suboptimal.
  • In WyScout v2: The orginal RecoveryEvent gets overwritten by the InterceptionEvent, whenever possible

What do you think? @probberechts @JanVanHaaren

Copy link
Contributor

@probberechts probberechts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've only added two suggestions. Apart from these it seems ok to me. Thanks for implementing this!

Note that I did not check the Wyscout v3 implementation in detail since I am not familiar with that data.

@@ -761,6 +792,14 @@ def deserialize(self, inputs: OptaInputs) -> EventDataset:
**duel_event_kwargs,
**generic_event_kwargs,
)
elif type_id == EVENT_TYPE_INTERCEPTION:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opta also has a "blocked pass" event. Would it make sense to parse it as an interception too?

blocked pass := When a player tries to cut out an opposition pass by any means. Similar to an interception except there is much less reading of the pass.

@@ -477,6 +510,24 @@ def deserialize(self, inputs: WyscoutInputs) -> EventDataset:
**generic_event_args,
)

# If also an interception event, add this before other event (It is a tag on multiple wyscout_events)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wyscout does not have a separate event type for interceptions. Instead, interceptions are recorded as a pass, duel or touch event with an "interception" tag. Your current implementation works well for passes, but for consistency with other providers (and socceraction) I would convert these duels/touch events to an interception. Not to an interception + duel.

@MKlaasman
Copy link
Contributor Author

MKlaasman commented Nov 14, 2023

Addition:

  1. Wyscout v2: Now duels and touches are overwritten with InterceptionEvent when they have an interception tag.
  2. Opta: Handle a blocked pass as an interception.
  3. Opta: Filtered deleted events upfront, otherwise next_event implementation prone for wrong InterceptionResult. Now, not logging the Skipping of events anymore. Someone, thinking this is a bad design choice?

Copy link
Collaborator

@JanVanHaaren JanVanHaaren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request looks good to me. Thanks!

@koenvo
Copy link
Contributor

koenvo commented Nov 15, 2023

@MKlaasman thanks for your work. Can you please merge master in? There are some small merge conflicts due to changes from Pieters PRs.

# Conflicts:
#	kloppy/infra/serializers/event/statsbomb/deserializer.py
#	kloppy/tests/test_helpers.py
#	kloppy/tests/test_state_builder.py
#	kloppy/tests/test_statsbomb.py
#	kloppy/tests/test_to_records.py
@koenvo koenvo merged commit e29808c into PySport:master Nov 16, 2023
19 checks passed
@koenvo koenvo added this to the 3.14 - Pi milestone Dec 29, 2023
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.

Add a data model for interceptions
4 participants