Bring in Ergast drivers in races only #699
Conversation
|
One example I found for a missing driver being pulled in from Ergast is Mick Schumacher at the 2022 Saudi Arabian Grand Prix. He did not start due to a heavy crash in Quali, I think. But he also isn't even listed in the official classification, contrary to Tsunoda who is listed as DNS after a mechanical issue before the race. Ergast lists Schumacher as "Withdrew". |
|
In that case, how do you want the code to behave? I can see arguments both for including and excluding Mick. Excluding him to stay consistent with official F1 classification, and including him to be more informative |
Casper-Guo
force-pushed
the
Fix-extra-practice-drivers
branch
3 times, most recently
from
1873392 to
6a60f11
Compare
|
Added tests, moved some code around a little bit, and improved (personal opinion) variable name. Ready for review now |
fastf1/core.py
Outdated
| if missing_drivers: | ||
| self._results.loc[missing_drivers, info_cols] \ | ||
| = driver_info_ergast.loc[missing_drivers, info_cols] | ||
| if self.name == "Race": |
There was a problem hiding this comment.
Actually, I was right about this after all. I was confused😅 We can and should use self.name in self._RACE_LIKE_SESSIONS. Further up, self._driver_results_from_ergast returns race, quali and sprint results for the corresponding sessions. It returns None for Sprint Qualifiying and the race results for a practice session. To be fair, a comment or docstring would have been appropriate there, when I implemented this.
There was a problem hiding this comment.
Is the support for all race-like sessions new in Jolpica? I thought Ergast only had the GPs.
I will add some explanatory comments for that
There was a problem hiding this comment.
This will be tough to test for. A driver has to be in qualifying to participate in GP and I cannot remember a driver change between sprint and race off the top of my head. I will look into that more.
There was a problem hiding this comment.
Ergast supports sprint races. It's missing sprint qualifying. For example: https://ergast.com/api/f1/2024/5/sprint/
We might be covering against a case that has never happened before. The logic is exactly the same as for the case where MSC is added, no? I'd be absolutely happy with just having the test for that. Mocking all the necessary data to make a mock test for the other case will be a lot of work for a feature that'll maybe never be needed.
There was a problem hiding this comment.
Agreed that mocking is too much work to be worth it. There is only two scenarios for the missing driver handling depending on whether Ergast gives us reliable driver lists or not. The test I added cover both of those so we should be good.
Based on your description, we really should be using self.name in self._RACE_LIKE_SESSIONS or self.name == "Qualifying" instead of just self.name in self._RACE_LIKE_SESSIONS no?
Casper-Guo
force-pushed
the
Fix-extra-practice-drivers
branch
from
6a60f11 to
b768388
Compare
Fixes #680