diff --git a/kloppy/infra/serializers/event/wyscout/deserializer_v2.py b/kloppy/infra/serializers/event/wyscout/deserializer_v2.py index 2542fa589..09821c4a4 100644 --- a/kloppy/infra/serializers/event/wyscout/deserializer_v2.py +++ b/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( diff --git a/kloppy/infra/serializers/event/wyscout/deserializer_v3.py b/kloppy/infra/serializers/event/wyscout/deserializer_v3.py index 546747278..5c91b9ec2 100644 --- a/kloppy/infra/serializers/event/wyscout/deserializer_v3.py +++ b/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,19 +700,6 @@ 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 = [] @@ -719,9 +707,9 @@ def create_periods(raw_events, period_minutes_offset_mapping): 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 diff --git a/kloppy/infra/serializers/event/wyscout/wyscout_periods.py b/kloppy/infra/serializers/event/wyscout/wyscout_periods.py new file mode 100644 index 000000000..457d27de9 --- /dev/null +++ b/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 diff --git a/kloppy/tests/prs/pr_609/test_pr_609.py b/kloppy/tests/prs/pr_609/test_pr_609.py new file mode 100644 index 000000000..a58c12e7f --- /dev/null +++ b/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" diff --git a/kloppy/tests/prs/pr_609/wyscout_events_v2_extra_time.json b/kloppy/tests/prs/pr_609/wyscout_events_v2_extra_time.json new file mode 100644 index 000000000..40d5edd37 --- /dev/null +++ b/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" + } + } + ] + } +} diff --git a/kloppy/tests/prs/pr_609/wyscout_events_v2_freekick_shot_last_event.json b/kloppy/tests/prs/pr_609/wyscout_events_v2_freekick_shot_last_event.json new file mode 100644 index 000000000..00f9f365a --- /dev/null +++ b/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" + } + } + ] + } +} diff --git a/kloppy/tests/prs/pr_609/wyscout_events_v2_null_roster.json b/kloppy/tests/prs/pr_609/wyscout_events_v2_null_roster.json new file mode 100644 index 000000000..2d483be29 --- /dev/null +++ b/kloppy/tests/prs/pr_609/wyscout_events_v2_null_roster.json @@ -0,0 +1,64 @@ +{ + "events": [ + { + "id": 1, + "playerId": 100, + "teamId": 1, + "matchId": 2499738, + "matchPeriod": "1H", + "eventSec": 10.0, + "eventName": 8, + "subEventName": 85, + "positions": [ + { + "x": 50.0, + "y": 50.0 + }, + { + "x": 60.0, + "y": 60.0 + } + ], + "tags": [ + { + "id": 1801 + } + ] + } + ], + "teams": { + "1": { + "team": { + "wyId": 1, + "officialName": "Team A" + } + }, + "2": { + "team": { + "wyId": 2, + "officialName": "Team B" + } + } + }, + "players": { + "1": [ + null, + { + "player": { + "wyId": 100, + "firstName": "Real", + "lastName": "Player" + } + } + ], + "2": [ + { + "player": { + "wyId": 101, + "firstName": "Other", + "lastName": "Player" + } + } + ] + } +} diff --git a/kloppy/tests/prs/pr_609/wyscout_events_v2_shot_last_event.json b/kloppy/tests/prs/pr_609/wyscout_events_v2_shot_last_event.json new file mode 100644 index 000000000..070281af1 --- /dev/null +++ b/kloppy/tests/prs/pr_609/wyscout_events_v2_shot_last_event.json @@ -0,0 +1,59 @@ +{ + "events": [ + { + "id": 1, + "playerId": 100, + "teamId": 1, + "matchId": 1694433, + "matchPeriod": "2H", + "eventSec": 5400.0, + "eventName": 10, + "subEventName": 100, + "positions": [ + { + "x": 90.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": "Shooter", + "lastName": "One" + } + } + ], + "2": [ + { + "player": { + "wyId": 101, + "firstName": "Goalie", + "lastName": "Two" + } + } + ] + } +}