[General] Fixed Undersirable "None" value for positions.last()

[UnravelSports]

Hi,

I noticed that with the new TimeContainer functionality, when we call player.positions.last() we get a None value as the last position for a lot of players. I'm assuming this is because of substitutions?

I've resolved the issue by now simply returning the last not None value, such that at least for every player that was on the pitch during the game the last() call gives a valid position.

Additionally, since I was getting a deprecation warning I fixed some very small syntax in the f24 xml parsing. This is not part of the same problem though.

Edit: I'll fix the tests if we agree that this problem even needs a fix

[koenvo]

The substitutions are an issue indeed. I would like to have a way to get the last value (including None in case the player is substituted). Maybe add a flag/argument to enable the new behaviour?

[UnravelSports]

It's been a while, you mean add an argument to the .last() call? Something like include_none?

[koenvo]

It's been a while, you mean add an argument to the .last() call? Something like include_none?

Yes indeed

[UnravelSports]

@koenvo should be fixed now

[koenvo]

This will break when there a only None values in self.items, correct? What should happen when time get a value of None? Maybe similar to https://github.com/PySport/kloppy/pull/374/files#diff-634d7febc3eb18e4cadff92f381df2b9125bec36feef3cea5e08762b4fab9d3cR255

[UnravelSports]

I have to admit I don't quite follow what you're saying.

If self.items.keys()[-1] returns None we have a different problem right? Because that means some how, somewhere a position change at time None was introduced. Is that possible right now?

Anyway, if we want to deal with that than what would be the expected return value of last() if time = None?

Would this make sense?

if include_time and time is not None:
            return time, self.items[time]
        elif not include_time:
            return self.items[time]
        else:
            return None