Fix Wyscout V2 deserializer: extra-time periods, final-event shots, n…

kloppy/infra/serializers/event/wyscout/deserializer_v2.py

@@ -38,6 +38,7 @@
 
 from ..deserializer import EventDataDeserializer
 from . import wyscout_events, wyscout_tags
+from .wyscout_periods import parse_period_id
 
 logger = logging.getLogger(__name__)
 
@@ -60,6 +61,7 @@ def _parse_team(raw_events, wyId: str, ground: Ground) -> Team:
             last_name=player["player"]["lastName"],
         )
         for player in raw_events["players"][wyId]
+        if player is not None
     ]
     return team
 
@@ -184,13 +186,16 @@ def _parse_shot(raw_event: dict, next_event: dict) -> dict:
     elif any(_has_tag(raw_event, tag) for tag in wyscout_tags.SHOT_ON_GOAL):
         result = ShotResult.SAVED
 
-    if next_event["eventId"] == wyscout_events.SAVE.EVENT:
-        if next_event["subEventId"] == wyscout_events.SAVE.REFLEXES:
-            qualifiers.append(GoalkeeperQualifier(GoalkeeperActionType.REFLEX))
-        if next_event["subEventId"] == wyscout_events.SAVE.SAVE_ATTEMPT:
-            qualifiers.append(
-                GoalkeeperQualifier(GoalkeeperActionType.SAVE_ATTEMPT)
-            )
+    if next_event:
+        if next_event["eventId"] == wyscout_events.SAVE.EVENT:
+            if next_event["subEventId"] == wyscout_events.SAVE.REFLEXES:
+                qualifiers.append(
+                    GoalkeeperQualifier(GoalkeeperActionType.REFLEX)
+                )
+            if next_event["subEventId"] == wyscout_events.SAVE.SAVE_ATTEMPT:
+                qualifiers.append(
+                    GoalkeeperQualifier(GoalkeeperActionType.SAVE_ATTEMPT)
+                )
 
     return {
         "result": result,
@@ -498,13 +503,11 @@ def _deserialize(self, inputs: WyscoutInputs) -> EventDataset:
                 next_period_id = None
                 if (idx + 1) < len(raw_events["events"]):
                     next_event = raw_events["events"][idx + 1]
-                    next_period_id = int(
-                        next_event["matchPeriod"].replace("H", "")
-                    )
+                    next_period_id = parse_period_id(next_event["matchPeriod"])
 
                 team_id = str(raw_event["teamId"])
                 player_id = str(raw_event["playerId"])
-                period_id = int(raw_event["matchPeriod"].replace("H", ""))
+                period_id = parse_period_id(raw_event["matchPeriod"])
 
                 if len(periods) == 0 or periods[-1].id != period_id:
                     periods.append(

kloppy/infra/serializers/event/wyscout/deserializer_v3.py

@@ -46,6 +46,7 @@
 
 from ..deserializer import EventDataDeserializer
 from .deserializer_v2 import WyscoutInputs
+from .wyscout_periods import parse_period_id
 
 logger = logging.getLogger(__name__)
 
@@ -455,12 +456,12 @@ def _parse_carry(raw_event: dict, next_event: dict, start_ts: dict) -> dict:
     )
 
     if next_event is not None:
-        period_id = _parse_period_id(next_event["matchPeriod"])
+        period_id = parse_period_id(next_event["matchPeriod"])
         end_timestamp = _create_timestamp_timedelta(
             next_event, start_ts, period_id
         )
     else:
-        period_id = _parse_period_id(raw_event["matchPeriod"])
+        period_id = parse_period_id(raw_event["matchPeriod"])
         end_timestamp = _create_timestamp_timedelta(
             raw_event, start_ts, period_id
         )
@@ -699,29 +700,16 @@ def _players_to_dict(players: list[Player]):
     return {player.player_id: player for player in players}
 
 
-def _parse_period_id(raw_period: str) -> int:
-    if "H" in raw_period:
-        period_id = int(raw_period.replace("H", ""))
-    elif "E" in raw_period:
-        period_id = 2 + int(raw_period.replace("E", ""))
-    elif raw_period == "P":
-        period_id = 5
-    else:
-        raise DeserializationError(f"Unknown period {raw_period}")
-
-    return period_id
-
-
 def create_periods(raw_events, period_minutes_offset_mapping):
     periods = []
 
     for idx, raw_event in enumerate(raw_events["events"]):
         next_period_id = None
         if (idx + 1) < len(raw_events["events"]):
             next_event = raw_events["events"][idx + 1]
-            next_period_id = _parse_period_id(next_event["matchPeriod"])
+            next_period_id = parse_period_id(next_event["matchPeriod"])
 
-        period_id = _parse_period_id(raw_event["matchPeriod"])
+        period_id = parse_period_id(raw_event["matchPeriod"])
 
         if len(periods) == 0 or periods[-1].id != period_id:
             periods.append(
@@ -836,7 +824,7 @@ def _deserialize(self, inputs: WyscoutInputs) -> EventDataset:
                 team_id = str(raw_event["team"]["id"])
                 team = teams[team_id]
                 player_id = str(raw_event["player"]["id"])
-                period_id = _parse_period_id(raw_event["matchPeriod"])
+                period_id = parse_period_id(raw_event["matchPeriod"])
 
                 if player_id == INVALID_PLAYER:
                     player = None

kloppy/infra/serializers/event/wyscout/wyscout_periods.py

@@ -0,0 +1,13 @@
+from kloppy.exceptions import DeserializationError
+
+
+def parse_period_id(raw_period: str) -> int:
+    if "H" in raw_period:
+        period_id = int(raw_period.replace("H", ""))
+    elif "E" in raw_period:
+        period_id = 2 + int(raw_period.replace("E", ""))
+    elif raw_period == "P":
+        period_id = 5
+    else:
+        raise DeserializationError(f"Unknown period {raw_period}")
+    return period_id

kloppy/tests/prs/pr_609/test_pr_609.py

@@ -0,0 +1,96 @@
+from kloppy import wyscout
+from kloppy.domain import EventType, SetPieceType
+
+
+def test_extra_time_period_parsing(base_dir):
+    """Test that extra-time period codes ("E1", "E2") are parsed correctly.
+
+    Regression test for issue #609 — 10 matches failed due to E1/E2 crashing int().
+    Real failing match: 1694426.
+    """
+    dataset = wyscout.load(
+        event_data=base_dir
+        / "prs"
+        / "pr_609"
+        / "wyscout_events_v2_extra_time.json",
+        coordinates="wyscout",
+        data_version="V2",
+    )
+
+    assert len(dataset.events) == 2
+    assert len(dataset.metadata.periods) == 2
+
+    period_ids = [p.id for p in dataset.metadata.periods]
+    assert 3 in period_ids
+    assert dataset.metadata.periods[1].id == 3
+
+
+def test_shot_as_last_event(base_dir):
+    """Test that a shot event as the last event in a match doesn't crash.
+
+    Regression test for issue #609 — 6 matches failed when _parse_shot tried to
+    access next_event without checking if it was None.
+    Real failing matches: 1694433, 2516925.
+    """
+    dataset = wyscout.load(
+        event_data=base_dir
+        / "prs"
+        / "pr_609"
+        / "wyscout_events_v2_shot_last_event.json",
+        coordinates="wyscout",
+        data_version="V2",
+    )
+
+    assert len(dataset.events) == 1
+    last_event = dataset.events[0]
+    assert last_event.event_type == EventType.SHOT
+
+
+def test_freekick_shot_as_last_event(base_dir):
+    """Test that a free-kick shot as the last event doesn't crash.
+
+    Verifies the fix for _parse_shot covers the _parse_set_piece call path.
+    Real failing match reference: 2516925.
+    """
+    dataset = wyscout.load(
+        event_data=base_dir
+        / "prs"
+        / "pr_609"
+        / "wyscout_events_v2_freekick_shot_last_event.json",
+        coordinates="wyscout",
+        data_version="V2",
+    )
+
+    assert len(dataset.events) == 1
+    shot_event = dataset.events[0]
+    assert shot_event.event_type == EventType.SHOT
+
+    set_piece_qualifiers = [
+        q
+        for q in shot_event.qualifiers
+        if hasattr(q, "value") and isinstance(q.value, SetPieceType)
+    ]
+    assert len(set_piece_qualifiers) == 1
+    assert set_piece_qualifiers[0].value == SetPieceType.FREE_KICK
+
+
+def test_null_roster_entries_skipped(base_dir):
+    """Test that null entries in a team's roster are silently skipped.
+
+    Regression test for issue #609 — 22 matches failed when some teams had
+    null entries in their players array (upstream data issue).
+    Real failing match: 2499738.
+    """
+    dataset = wyscout.load(
+        event_data=base_dir
+        / "prs"
+        / "pr_609"
+        / "wyscout_events_v2_null_roster.json",
+        coordinates="wyscout",
+        data_version="V2",
+    )
+
+    home_team = dataset.metadata.teams[0]
+    assert len(home_team.players) == 1
+    assert home_team.players[0].player_id == "100"
+    assert home_team.players[0].first_name == "Real"

kloppy/tests/prs/pr_609/wyscout_events_v2_extra_time.json

@@ -0,0 +1,88 @@
+{
+  "events": [
+    {
+      "id": 1,
+      "playerId": 100,
+      "teamId": 1,
+      "matchId": 1694426,
+      "matchPeriod": "2H",
+      "eventSec": 5400.0,
+      "eventName": 8,
+      "subEventName": 85,
+      "positions": [
+        {
+          "x": 50.0,
+          "y": 50.0
+        },
+        {
+          "x": 60.0,
+          "y": 60.0
+        }
+      ],
+      "tags": [
+        {
+          "id": 1801
+        }
+      ]
+    },
+    {
+      "id": 2,
+      "playerId": 101,
+      "teamId": 2,
+      "matchId": 1694426,
+      "matchPeriod": "E1",
+      "eventSec": 5420.0,
+      "eventName": 8,
+      "subEventName": 85,
+      "positions": [
+        {
+          "x": 60.0,
+          "y": 60.0
+        },
+        {
+          "x": 70.0,
+          "y": 70.0
+        }
+      ],
+      "tags": [
+        {
+          "id": 1801
+        }
+      ]
+    }
+  ],
+  "teams": {
+    "1": {
+      "team": {
+        "wyId": 1,
+        "officialName": "Team A"
+      }
+    },
+    "2": {
+      "team": {
+        "wyId": 2,
+        "officialName": "Team B"
+      }
+    }
+  },
+  "players": {
+    "1": [
+      {
+        "player": {
+          "wyId": 100,
+          "firstName": "Player",
+          "lastName": "One"
+        }
+      }
+    ],
+    "2": [
+      {
+        "player": {
+          "wyId": 101,
+          "firstName": "Player",
+          "lastName": "Two"
+        }
+      }
+    ]
+  }
+}

kloppy/tests/prs/pr_609/wyscout_events_v2_freekick_shot_last_event.json

@@ -0,0 +1,59 @@
+{
+  "events": [
+    {
+      "id": 1,
+      "playerId": 100,
+      "teamId": 1,
+      "matchId": 2516925,
+      "matchPeriod": "2H",
+      "eventSec": 5400.0,
+      "eventName": 3,
+      "subEventName": 33,
+      "positions": [
+        {
+          "x": 85.0,
+          "y": 50.0
+        }
+      ],
+      "tags": [
+        {
+          "id": 1801
+        }
+      ]
+    }
+  ],
+  "teams": {
+    "1": {
+      "team": {
+        "wyId": 1,
+        "officialName": "Team A"
+      }
+    },
+    "2": {
+      "team": {
+        "wyId": 2,
+        "officialName": "Team B"
+      }
+    }
+  },
+  "players": {
+    "1": [
+      {
+        "player": {
+          "wyId": 100,
+          "firstName": "FreeKick",
+          "lastName": "Taker"
+        }
+      }
+    ],
+    "2": [
+      {
+        "player": {
+          "wyId": 101,
+          "firstName": "Goalie",
+          "lastName": "Three"
+        }
+      }
+    ]
+  }
+}