Repository: PySport/kloppy

Issue:
Upgrading lxml dependency to +5.0.0
Hi,

Quick question. In some projects I want to install both `kloppy` and `floodlight`, but there is a dependency issue, because kloppy relies on <5 and floodlight depends on lxml >5. 

I've just done a basic test by installing the latest version of lxml and running all the kloppy tests successfully.

Would anything else be required to upgrade this dependency?

Discussion:

Solution patch:
diff --git a/kloppy/infra/serializers/event/statsbomb/specification.py b/kloppy/infra/serializers/event/statsbomb/specification.py
index 32a6370..536e85f 100644
--- a/kloppy/infra/serializers/event/statsbomb/specification.py
+++ b/kloppy/infra/serializers/event/statsbomb/specification.py
@@ -1274,6 +1274,18 @@ class BALL_RECOVERY(EVENT):
     def _create_events(
         self, event_factory: EventFactory, **generic_event_kwargs
     ) -> List[Event]:
+        recovery_dict = self.raw_event.get("ball_recovery", {})
+        recovery_failure = recovery_dict.get("recovery_failure", False)
+        if recovery_failure:
+            duel_event = event_factory.build_duel(
+                result=DuelResult.LOST,
+                qualifiers=[
+                    DuelQualifier(value=DuelType.LOOSE_BALL),
+                ],
+                **generic_event_kwargs,
+            )
+            return [duel_event]
+
         recovery_event = event_factory.build_recovery(
             result=None,
             qualifiers=None,
diff --git a/kloppy/infra/serializers/event/statsperform/deserializer.py b/kloppy/infra/serializers/event/statsperform/deserializer.py
index 47d6b4a..b897ae3 100644
--- a/kloppy/infra/serializers/event/statsperform/deserializer.py
+++ b/kloppy/infra/serializers/event/statsperform/deserializer.py
@@ -1,49 +1,50 @@
-import math
-from typing import Dict, List, NamedTuple, IO, Optional
 import logging
+import math
 from datetime import datetime, timedelta
+from typing import IO, Dict, List, NamedTuple, Optional
 
 import pytz
 
 from kloppy.domain import (
-    EventDataset,
-    Team,
-    Point,
-    Point3D,
     BallState,
+    BodyPart,
+    BodyPartQualifier,
+    CardQualifier,
+    CardType,
+    CounterAttackQualifier,
     DatasetFlag,
-    Orientation,
-    PassResult,
-    ShotResult,
-    TakeOnResult,
+    DuelQualifier,
     DuelResult,
     DuelType,
-    DuelQualifier,
-    Provider,
-    Metadata,
+    EventDataset,
+    ExpectedGoals,
+    GoalkeeperActionType,
+    GoalkeeperQualifier,
     InterceptionResult,
-    CardType,
-    CardQualifier,
-    Qualifier,
-    SetPieceQualifier,
-    SetPieceType,
-    BodyPartQualifier,
-    BodyPart,
-    PassType,
+    Metadata,
+    Orientation,
     PassQualifier,
-    GoalkeeperQualifier,
-    GoalkeeperActionType,
-    CounterAttackQualifier,
+    PassResult,
+    PassType,
+    Period,
+    Point,
+    Point3D,
     PositionType,
-    ExpectedGoals,
     PostShotExpectedGoals,
+    Provider,
+    Qualifier,
+    SetPieceQualifier,
+    SetPieceType,
+    ShotResult,
+    TakeOnResult,
+    Team,
 )
 from kloppy.exceptions import DeserializationError
 from kloppy.infra.serializers.event.deserializer import EventDataDeserializer
 from kloppy.utils import performance_logging
 
-from .parsers import get_parser, OptaEvent
-from .formation_mapping import formation_position_mapping, formation_id_mapping
+from .formation_mapping import formation_id_mapping, formation_position_mapping
+from .parsers import OptaEvent, get_parser
 
 logger = logging.getLogger(__name__)
 
@@ -157,6 +158,8 @@ EVENT_QUALIFIER_FORMATION_PLAYER_POSITIONS = 131
 EVENT_QUALIFIER_XG = 321
 EVENT_QUALIFIER_POST_SHOT_XG = 322
 
+EVENT_TYPE_CHALLENGE = 45
+
 event_type_names = {
     1: "pass",
     2: "offside pass",
@@ -246,38 +249,71 @@ position_line_mapping = {
 
 
 def _parse_pass(
-    raw_event: OptaEvent, next_event: OptaEvent, next_next_event: OptaEvent
+    raw_event: OptaEvent,
+    next_event: OptaEvent,
+    next_next_event: OptaEvent,
+    team: Team,
+    period: Period,
 ) -> Dict:
-    if raw_event.outcome:
-        result = PassResult.COMPLETE
-    else:
-        result = PassResult.INCOMPLETE
+    result = (
+        PassResult.COMPLETE if raw_event.outcome else PassResult.INCOMPLETE
+    )
     receiver_coordinates = _get_end_coordinates(raw_event.qualifiers)
-    pass_qualifiers = _get_pass_qualifiers(raw_event.qualifiers)
-    overall_qualifiers = _get_event_qualifiers(raw_event.qualifiers)
+    qualifiers = _get_pass_qualifiers(
+        raw_event.qualifiers
+    ) + _get_event_qualifiers(raw_event.qualifiers)
 
-    qualifiers = pass_qualifiers + overall_qualifiers
+    # Look for a ball receipt event based on the result and sequence of events
+    ball_receipt_event = None
+    if result == PassResult.COMPLETE and next_event is not None:
+        if (
+            next_event.contestant_id == team.team_id
+            and next_event.type_id in BALL_OWNING_EVENTS
+        ):
+            ball_receipt_event = next_event
+        elif (
+            next_event.type_id == EVENT_TYPE_CHALLENGE
+            and next_next_event is not None
+            and next_next_event.contestant_id == team.team_id
+            and next_next_event.type_id in BALL_OWNING_EVENTS
+        ):
+            ball_receipt_event = next_next_event
+    # Assign the receiver_player and receive_timestamp
+    # if a ball_receipt_event was found
+    if (
+        ball_receipt_event is not None
+        and ball_receipt_event.player_id is not None
+    ):
+        receiver_player = team.get_player_by_id(ball_receipt_event.player_id)
+        receive_timestamp = (
+            ball_receipt_event.timestamp - period.start_timestamp
+        )
+    else:
+        receiver_player = None
+        receive_timestamp = None
 
     # Set the end location of a deflected pass to the start location
     # of the next action and the outcome to "success" if the deflected
     # pass reached a teammate
-    if next_event is not None and next_next_event is not None:
-        if (
-            next_event.type_id == EVENT_TYPE_BALL_TOUCH
-            and next_event.outcome == 1
-            and next_next_event.contestant_id == raw_event.contestant_id
-        ):
-            result = PassResult.COMPLETE
-            receiver_coordinates = Point(
-                x=next_next_event.x,
-                y=next_next_event.y,
-            )
+    if (
+        next_event is not None
+        and next_next_event is not None
+        and next_event.type_id == EVENT_TYPE_BALL_TOUCH
+        and next_event.outcome == 1
+        and next_next_event.contestant_id == raw_event.contestant_id
+    ):
+        result = PassResult.COMPLETE
+        receiver_coordinates = Point(
+            x=next_next_event.x,
+            y=next_next_event.y,
+        )
+        receive_timestamp = next_next_event.timestamp - period.start_timestamp
 
     return dict(
         result=result,
         receiver_coordinates=receiver_coordinates,
-        receiver_player=None,
-        receive_timestamp=None,
+        receiver_player=receiver_player,
+        receive_timestamp=receive_timestamp,
         qualifiers=qualifiers,
     )
 
@@ -785,7 +821,7 @@ class StatsPerformDeserializer(EventDataDeserializer[StatsPerformInputs]):
                         ball_owning_team=possession_team,
                         ball_state=ball_state,
                         # from Event
-                        event_id=raw_event.id,
+                        event_id=str(raw_event.id),
                         team=team,
                         player=player,
                         coordinates=Point(x=raw_event.x, y=raw_event.y),
@@ -794,7 +830,11 @@ class StatsPerformDeserializer(EventDataDeserializer[StatsPerformInputs]):
 
                     if raw_event.type_id == EVENT_TYPE_PASS:
                         pass_event_kwargs = _parse_pass(
-                            raw_event, next_event, next_next_event
+                            raw_event,
+                            next_event,
+                            next_next_event,
+                            team,
+                            period,
                         )
                         event = self.event_factory.build_pass(
                             **pass_event_kwargs,
diff --git a/setup.py b/setup.py
index a2ed974..5bbb520 100644
--- a/setup.py
+++ b/setup.py
@@ -38,7 +38,7 @@ def setup_package():
             "console_scripts": ["kloppy-query = kloppy.cmdline:run_query"]
         },
         install_requires=[
-            "lxml>=4.4.0,<5",
+            "lxml>=4.4.0",
             "requests>=2.0.0,<3",
             "pytz>=2020.1",
             'typing_extensions;python_version<"3.11"',


Test patch:
diff --git a/kloppy/tests/test_state_builder.py b/kloppy/tests/test_state_builder.py
index d27f619..10326d5 100644
--- a/kloppy/tests/test_state_builder.py
+++ b/kloppy/tests/test_state_builder.py
@@ -1,9 +1,9 @@
 from itertools import groupby
 
-from kloppy.domain import EventType, Event, EventDataset, FormationType
+from kloppy import statsbomb
+from kloppy.domain import Event, EventDataset, EventType, FormationType
 from kloppy.domain.services.state_builder.builder import StateBuilder
 from kloppy.utils import performance_logging
-from kloppy import statsbomb
 
 
 class TestStateBuilder:
@@ -51,7 +51,7 @@ class TestStateBuilder:
             events_per_sequence[sequence_id] = len(events)
 
         assert events_per_sequence[0] == 4
-        assert events_per_sequence[51] == 7
+        assert events_per_sequence[51] == 10
 
     def test_lineup_state_builder(self, base_dir):
         dataset = self._load_dataset(base_dir, base_filename="statsbomb_15986")
diff --git a/kloppy/tests/test_statsbomb.py b/kloppy/tests/test_statsbomb.py
index 470a003..19f7938 100644
--- a/kloppy/tests/test_statsbomb.py
+++ b/kloppy/tests/test_statsbomb.py
@@ -957,8 +957,8 @@ class TestStatsBombDuelEvent:
         """It should deserialize all duel and 50/50 events"""
         events = dataset.find_all("duel")
         assert (
-            len(events) == 59 + 4 + 26
-        )  # duels + 50/50 + aerial won attribute
+            len(events) == 59 + 4 + 26 + 4
+        )  # duels + 50/50 + aerial won attribute + failed recoveries
 
     def test_attributes(self, dataset: EventDataset):
         """Verify specific attributes of duels"""
@@ -1171,10 +1171,21 @@ class TestStatsBombPlayerOnEvent:
 class TestStatsBombRecoveryEvent:
     """Tests related to deserializing 23/Recovery events"""
 
-    def test_deserialize_all(self, dataset: EventDataset):
-        """It should deserialize all ball recovery events"""
+    def test_deserialize_successful(self, dataset: EventDataset):
+        """It should deserialize all successful ball recovery events"""
         events = dataset.find_all("recovery")
-        assert len(events) == 97
+        assert len(events) == 93
+
+    def test_deserialize_failed(self, dataset: EventDataset):
+        """It should deserialize all failed ball recovery events as loose ball duels"""
+        failed_recovery = dataset.get_event_by_id(
+            "0df4c1d6-1c4a-407b-876d-d9ac80fd7eee"
+        )
+        assert failed_recovery.event_type == EventType.DUEL
+        assert failed_recovery.get_qualifier_values(DuelQualifier) == [
+            DuelType.LOOSE_BALL,
+        ]
+        assert failed_recovery.result == DuelResult.LOST
 
 
 class TestStatsBombTacticalShiftEvent:
diff --git a/kloppy/tests/test_statsperform.py b/kloppy/tests/test_statsperform.py
index a4d723e..38af05e 100644
--- a/kloppy/tests/test_statsperform.py
+++ b/kloppy/tests/test_statsperform.py
@@ -9,13 +9,14 @@ from kloppy.domain import (
     EventDataset,
     OptaCoordinateSystem,
     Orientation,
+    PassResult,
     Point,
     Point3D,
+    PositionType,
     Provider,
     SportVUCoordinateSystem,
-    TrackingDataset,
     Time,
-    PositionType,
+    TrackingDataset,
 )
 
 
@@ -95,6 +96,13 @@ class TestStatsPerformMetadata:
     def test_provider(self, tracking_dataset: TrackingDataset):
         assert tracking_dataset.metadata.provider == Provider.STATSPERFORM
 
+    def test_orientation(self, event_dataset: EventDataset):
+        assert event_dataset.metadata.coordinate_system == OptaCoordinateSystem(
+            # StatsPerform does not provide pitch dimensions
+            pitch_length=None,
+            pitch_width=None,
+        )
+
     def test_teams(self, tracking_dataset: TrackingDataset):
         home_team = tracking_dataset.metadata.teams[0]
         home_player = home_team.players[2]
@@ -176,11 +184,7 @@ class TestStatsPerformEvent:
 
     def test_deserialize_all(self, event_dataset: EventDataset):
         assert event_dataset.metadata.provider == Provider.STATSPERFORM
-        assert event_dataset.metadata.coordinate_system == OptaCoordinateSystem(
-            # StatsPerform does not provide pitch dimensions
-            pitch_length=None,
-            pitch_width=None,
-        )
+
         assert len(event_dataset.records) == 1643
 
         substitution_events = event_dataset.find_all("substitution")
@@ -202,6 +206,64 @@ class TestStatsPerformEvent:
         assert first_sub.player == m_wintzheimer
         assert first_sub.replacement_player == b_jatta
 
+    def test_pass_receiver(self, event_dataset: EventDataset):
+        """It should impute the intended receiver of a pass."""
+        # Completed passes should get a receiver
+        complete_pass = event_dataset.get_event_by_id("2328589789")
+        assert (
+            complete_pass.receiver_player.player_id
+            == "apdrig6xt1hxub1986s3uh1x"
+        )
+        assert (
+            complete_pass.receive_timestamp
+            == complete_pass.next_record.timestamp
+        )
+
+        # When a pass is challenged the receipt is the next next event
+        challenged_pass = event_dataset.get_event_by_id("2328600271")
+        assert (
+            challenged_pass.receiver_player.player_id
+            == "ci4pwzieoc94uj3i1371bsatx"
+        )
+        assert (
+            challenged_pass.receive_timestamp
+            == challenged_pass.next_record.next_record.timestamp
+        )
+
+        # Passes should be received within 30 seconds
+        assert all(
+            [
+                p.receive_timestamp.total_seconds()
+                - p.timestamp.total_seconds()
+                < 30
+                for p in event_dataset.find_all("pass")
+                if p.receive_timestamp is not None
+            ]
+        )
+
+        # Passes that result in a loss of possession should not have a receiver
+        turnover_passes = [
+            p
+            for p in event_dataset.find_all("pass")
+            if p.next_record
+            and p.ball_owning_team != p.next_record.ball_owning_team
+        ]
+        assert all(p.receiver_player is None for p in turnover_passes)
+
+        # Failed passes should not have a receiver
+        failed_pass = event_dataset.get_event_by_id("2328591011")
+        assert failed_pass.receiver_player is None
+        out_pass = event_dataset.get_event_by_id("2328590733")
+        assert out_pass.receiver_player is None
+
+        # When a pass is interrupted by a foul the receiver is not set
+        fouled_pass = event_dataset.get_event_by_id("2328589929")
+        assert fouled_pass.receiver_player is None
+
+        # Deflected passes should not have a receiver
+        deflected_pass = event_dataset.get_event_by_id("2328596237")
+        assert deflected_pass.receiver_player is None
+
 
 class TestStatsPerformTracking:
     """Tests related to deserializing tracking data delivered by StatsPerform."""