Event type qualifiers

[DriesDeprest]

As you can see in the code below, we add at maximum only one PassQualifier per pass event. @koenvo Why do we only add one PassQualifier per pass event? Why don't we change all the elif to if statements? Wouldn't that more accurately describe the pass event?

This question / idea is generalizable to any event type for any event provider.

Code:
https://github.com/PySport/kloppy/blob/master/kloppy/infra/serializers/event/opta/deserializer.py#LL386C1-L404C22

def _get_event_pass_qualifiers(raw_qualifiers: List) -> List[Qualifier]:
    qualifiers = []
    if EVENT_QUALIFIER_CROSS in raw_qualifiers:
        qualifiers.append(PassQualifier(value=PassType.CROSS))
    elif EVENT_QUALIFIER_LONG_BALL in raw_qualifiers:
        qualifiers.append(PassQualifier(value=PassType.LONG_BALL))
    elif EVENT_QUALIFIER_CHIPPED_BALL in raw_qualifiers:
        qualifiers.append(PassQualifier(value=PassType.CHIPPED_PASS))
    elif EVENT_QUALIFIER_THROUGH_BALL in raw_qualifiers:
        qualifiers.append(PassQualifier(value=PassType.THROUGH_BALL))
    elif EVENT_QUALIFIER_LAUNCH in raw_qualifiers:
        qualifiers.append(PassQualifier(value=PassType.LAUNCH))
    elif EVENT_QUALIFIER_FLICK_ON in raw_qualifiers:
        qualifiers.append(PassQualifier(value=PassType.FLICK_ON))
    elif EVENT_QUALIFIER_ASSIST in raw_qualifiers:
        qualifiers.append(PassQualifier(value=PassType.ASSIST))
    elif EVENT_QUALIFIER_ASSIST_2ND in raw_qualifiers:
        qualifiers.append(PassQualifier(value=PassType.ASSIST_2ND))
    return qualifiers

[koenvo]

Agree we can keep them all in the Event.

Right now there exists only Event.get_qualifier_value. When we add multiple values for a single qualifier it would be good to have a way to get all qualifiers for a certain event.

[DriesDeprest]

I'm in doubt what is the best approach to store multiple qualifiers of the same type (e.g. multiple PassQualifier) in one event.

1. Either we move from PassQualifier (contains a single value) to PassQualifiers (contains a list of values)
   Straw man proposal for PassQualifiers: https://github.com/my-game-plan/kloppy/tree/introduce-PassQualifiers

2. Either we store multiple PassQualifier's per event, but allow us to retrieve a list of qualifiers for Qualifier types which can have multiple values.
   Straw man proposal: https://github.com/my-game-plan/kloppy/tree/introduce-multiple-PassQualifier-s

I'm focusing now on PassQualifier(s) for the sake of an example, but the idea is generalizable to any Qualifier which can have multiple values.

@koenvo what do you think?

[DriesDeprest]

@JanVanHaaren @MKlaasman also interested to hear your take on this!

[JanVanHaaren]

I agree that we should support multiple Qualifier instances for an Event instance. However, since I have not used this functionality much myself, I have only a limited understanding of what would be an elegant solution.

In the first proposal, I was wondering whether we want the additional complexity of the PassQualifiers container object, especially since we do not use such a container object in the Event class and store the Qualifier objects in a list.

In the second proposal, I was wondering whether we really need the multiple_values flag. It is somewhat counter-intuitive to me that an instance of a class should be aware of the fact that multiple instances of that class can be present at the same time.

The following thoughts crossed my mind while looking at the proposals:

• Would it help to turn the qualifiers instance variable in the Event class from a list of Qualifier objects into a Qualifiers object that handles and hides the complexity of dealing with qualifiers from the rest of the Event class?
• Would it help to turn the list of Qualifier objects in the Event class into a dict where the keys are the qualifier types and the values are lists of Qualifier objects?
• Would it help to name the qualifiers in a more clever way? For instance, we could turn the name property in the Qualifier class into an abstract property that needs to be overriden by all concrete subclasses. I feel that this change could remove the need for the multiple_values flag in the second proposal.

[DriesDeprest]

Thanks for the input @JanVanHaaren. My understanding of this functionality is limited as well, so @koenvo could you maybe weigh in? I'm having trouble deciding what the 'ideal' way forward is.

I want to pick up this task again, as it could greatly reduce the complexity of how we handle the data model of a football event in my projects (and hopefully for others as well).

[koenvo]

It seems to current implementation already supports the above situation:

1. Some deserializers already allow multiple qualifiers of the same type like https://github.com/PySport/kloppy/blob/master/kloppy/infra/serializers/event/statsbomb/deserializer.py#L260-L294
2. There is also already a get_qualifier_values method on Event. We might replace the type of qualifiers: List[Qualifier] to qualifiers: Qualifiers where Qualifiers can extend list to make the usage convenient.

So I would suggest to keep the implementation as it currently is.

There are some questions that we might want to answer sometime in the future:

1. Do we need a different container type for qualifiers?
2. Should we define rules on which values can be combined? From PassType.ASSIST is being used both as a shot and goal assist interchangeably #239: ASSIST_SHOT and ASSIST_GOAL are mutually exclusive while ASSIST_SHOT could be combined with CROSS.