search

theOehrly / Fast-F1

FastF1 is a python package for accessing and analyzing Formula 1 results, schedules, timing data and telemetry
https://docs.fastf1.dev
MIT License
2.29k stars 239 forks source link

ENH: Declare Sprint Qualifying as a quali-like session #572

Closed pesaventofilippo closed 2 months ago

pesaventofilippo commented 2 months ago

Sprint Qualifying for 2024 is now a Quali-Like session (not Race-Like like before). Also fixes Laps.split_qualifying_sessions() for the Sprint Qualifying

theOehrly commented 2 months ago

Hi, this needs to be defined dependent on the season and can't just be changed globally. Else, we are breaking support for 2018.

My solution for this isn't perfect but ok, making these constants instance variables of the Session that are set in init, depending on the year.

If you have a better suggesting, I'll gladly take that.

Also, there is Norris' reinstated lap, which isn't working as well. So this will take a bit to fix overall, maybe.

theOehrly commented 2 months ago

And apparently, there is a keyboard shortcut to close PRs? 😅

pesaventofilippo commented 2 months ago

this needs to be defined dependent on the season and can't just be changed globally. Else, we are breaking support for 2018.

My bad, I thought this was a situation like with driver/color support where we only target the current year.

For now I also think the best solution is to change the behaviour depending on the year. There's not much else to do unfortunately if they reuse the same session name to mean different things in different years...

Also, there is Norris' reinstated lap, which isn't working as well.

Yes, a quick fix if anyone has problems is to use Laps.pick_fastest(only_by_time=True). I guess this is a problem with how the lap has been deleted and then reinstated, I can't recall other events where this has happened before. If someone remembers, we could test if this happens every time (for example, if a lap gets reinstated during the session or just after with a race control message?)

theOehrly commented 2 months ago

I think I have both things fixed. Need to check that all tests are still passing and that I haven't missed anything.

My bad, I thought this was a situation like with driver/color support where we only target the current year.

I'd prefer not to do that. And it seems straight forward enough to make it season dependent.

I guess this is a problem with how the lap has been deleted and then reinstated, I can't recall other events where this has happened before. If someone remembers, we could test if this happens every time

I don't remember another event, either. But I'd appreciate it if someone remembers, so I can use it for testing as well.

pesaventofilippo commented 2 months ago

I just have an update: see SessionInfo.json, where "Type" is "Qualifying".

I've tried with the 2023 Austrian GP, another sprint weekend with the old format: the Sprint Shootout was of type "Qualifying", and the Sprint Race was of type "Race". This seems promising I think, should it be a property of Session, and then instead of using QUALI_LIKE and RACE_LIKE we directly check for that value?

theOehrly commented 2 months ago

I just have an update: see SessionInfo.json, where "Type" is "Qualifying".

I've tried with the 2023 Austrian GP, another sprint weekend with the old format: the Sprint Shootout was of type "Qualifying", and the Sprint Race was of type "Race". This seems promising I think, should it be a property of Session, and then instead of using QUALI_LIKE and RACE_LIKE we directly check for that value?

That's not a bad idea, but I think I'd delay that. It would be great for cleaning up the code a bit and having less hard-coded values. But there are some things that need to be considered additionally.

I'd prefer to not make a change like that right now, as it's more involved, and I don't have the time right now to properly ensure that it works as intended in all scenarios. I'll go with the quick and dirty fix. But I'd really want to consider your suggestion for the future.

theOehrly commented 2 months ago

I'll close this PR as the immediate problem was resolved. Regarding the discussion about using the information from SessionInfo.json, I opened #581 so that this idea won't get lost.