search

PySport / kloppy

kloppy: standardizing soccer tracking- and event data
https://kloppy.pysport.org
BSD 3-Clause "New" or "Revised" License
365 stars 59 forks source link

Period equality in tests #107

Open ProformAnalytics opened 3 years ago

ProformAnalytics commented 3 years ago

The __eq__ method for Period is currently implemented as a check of equality of the id attributes. This makes sense as it makes it easy to compare between games, e.g. asking do these events from different games both happen in the first half?

However, this also potentially introduces errors in the tests where we want to check that a Period is identical to what is expected. For example, in test_tracab.py, the following assertion will pass for any value of start_timestamp etc provided the id is correct

assert dataset.metadata.periods[0] == Period(
    id=1,
    start_timestamp=4.0,
    end_timestamp=4.08,
    attacking_direction=AttackingDirection.HOME_AWAY,
)

This should be easy enough to fix, depending on how you guys think would be best to go about it?

koenvo commented 3 years ago

Sorry for my absence for a while. I believe Period shouldn't be identical over multiple matches. Current behaviour may result in mistakes when periods from different matches are mixed.

I'm not totally sure current code relies on id attribute only equality check.

@bdagnino do you agree on changing the behaviour?