Fix AttributeError when TeamColour is None in _load_drivers_from_f1_livetiming#932
Fix AttributeError when TeamColour is None in _load_drivers_from_f1_livetiming#932Federicorao wants to merge 3 commits into
Conversation
|
Your PR description is missing the required AI disclosure. Please review Fastf1's AI policy, and then update your PR to include a section titled AI Disclosure (verbatim). PRs without AI disclosures will not be reviewed. |
There was a problem hiding this comment.
Please add the required AI disclosure as requested.
The fix is not idiomatic in the context of Python. I also need to take a bit longer to look at what is actually happening with the request here to determine the appropriate fallback value. #000000 is almost certainly not a desirable team color given that users may be plotting on light background.
|
Updated this PR based on the review feedback.
Validated with focused plotting pytest coverage, |
| team_color = None | ||
| if team_color_raw: | ||
| team_color = f"#{team_color_raw.lower()}" | ||
| abbreviation = driver_entry.get('Tla', '') |
There was a problem hiding this comment.
team_color = None case should be handled here. This simplifies the logic flow and makes parsing the code much easier. I don't believe handling this condition in two separate code paths later is better/cleaner
| } | ||
| } | ||
|
|
||
| monkeypatch.setattr(fastf1._api, 'driver_info', mock_driver_info) |
There was a problem hiding this comment.
I assume this monkeypatch is introduced to limit the scope of setattr? In either case, I would like to avoid setattr unless it is well justified and the ramifications clearly documented
This makes sense.
Can you please explain the implication of this and how it differs from the old behavior? |
2 participants
Fixes #931.
What changed
TeamColour=None.TeamColour.Why
Some sessions include driver entries with
TeamColourset toNone. The previous logic called.lower()unconditionally; using#000000as a fallback is not desirable because it can produce incorrect plot colors. Known teams already have official color constants, so the missing live value should leave those constants intact.Verification
.venv/bin/python -m pytest fastf1/tests/test_plotting.py -k "load_drivers_preserves_known_team_color_if_live_color_missing".venv/bin/python -m pytest fastf1/tests/test_plotting.py -k "get_team_name_by_driver or load_drivers_preserves_known_team_color_if_live_color_missing".venv/bin/python -m ruff check fastf1/plotting/_backend.py.venv/bin/isort --check-only fastf1/plotting/_backend.py fastf1/tests/test_plotting.pyAI Disclosure
AI tools were used to inspect the issue and review feedback, implement the follow-up change, add the regression test, run local validation, and update this PR description. The submitted code changes are in
fastf1/plotting/_backend.pyandfastf1/tests/test_plotting.py.