Add statistics attribute to DataRecord #302
Conversation
|
It seems that for Python versions < 3.10, we get: TypeError: dataclass() got an unexpected keyword argument 'kw_only'. I'm not sure what the right way is to handle this, so any help is greatly appreciated, thanks. |
# Conflicts: # kloppy/infra/serializers/event/statsbomb/specification.py
|
@koenvo @JanVanHaaren this implementation works for you guys? |
| return event_cls(**relevant_kwargs) | ||
| event = event_cls(**relevant_kwargs) | ||
|
|
||
| statistics = all_kwargs.get("statistics", []) |
There was a problem hiding this comment.
I prefer this to be passed via kwargs, a couple of lines above. Like freeze_frame
There was a problem hiding this comment.
I'm not sure how it would work.
statistics: Optional[List[Statistic]] = field(
init=False, default_factory=list
)In my implementation, the statistics cannot be initialized on creation, but only after. Thus, we cannot pass it as kwargs on creation, right?
The reason why I have put init=False is because otherwise it complains about TypeError: non-default argument follows default argument
There was a problem hiding this comment.
A problem, however, with my current implementation is that the statistics of an event gets reset to [] when the dataclasses replace() method is being executed on an event as the result of e.g. a transformation.
Any idea how to properly handle the statistics?
There was a problem hiding this comment.
@koenvo could you review my latest commit? Thanks!
|
Nice. Some minor comments. |
|
Resolved the comments, can you check again? |
# Conflicts: # kloppy/infra/serializers/event/statsbomb/helpers.py # kloppy/infra/serializers/event/statsbomb/specification.py # kloppy/infra/serializers/event/statsperform/deserializer.py # kloppy/infra/serializers/tracking/statsperform.py # kloppy/infra/serializers/tracking/tracab/tracab_json.py
|
@koenvo merge conflicts are resolved! Ready to merge! |
Related to: #280.
I've started with adding support for xG statistics and wanted to get initial feedback on the implementation prior to expanding it to also parse OB values of StatsBomb and potentially other statistics.
So happy to hear your thoughts @probberechts @koenvo @JanVanHaaren