fix(frcteamrank): handle null qual data from The Blue Alliance API#418
fix(frcteamrank): handle null qual data from The Blue Alliance API#418tavdog merged 2 commits intotronbyt:mainfrom
Conversation
The Blue Alliance returns "qual": null when an event is active but matches haven't started yet. The existing checks only verified key existence, not whether the value was None, causing a crash on the `in` operator.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a robustness issue in the FRC Team Rank application by improving its handling of incomplete or null data returned by The Blue Alliance API. It ensures that the application can gracefully process event data even when qualification matches have not yet begun, preventing crashes and providing a consistent user experience. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly fixes a crash when The Blue Alliance API returns null for qualification data. The added checks for None values are appropriate. However, the implementation results in repetitive and deeply nested dictionary access, which impacts code readability and maintainability. A review comment has been added to suggest a refactoring that would simplify this logic.
apps/frcteamrank/frc_team_rank.star
Outdated
| if "qual" not in ranking_data or ranking_data["qual"] == None: | ||
| return -1, 999 | ||
| if "ranking" not in ranking_data["qual"]: | ||
| if "ranking" not in ranking_data["qual"] or ranking_data["qual"]["ranking"] == None: | ||
| return -1, 999 | ||
| if "rank" not in ranking_data["qual"]["ranking"]: | ||
| if "rank" not in ranking_data["qual"]["ranking"] or ranking_data["qual"]["ranking"]["rank"] == None: | ||
| return -1, 999 |
There was a problem hiding this comment.
The repeated deep access into the ranking_data dictionary reduces readability and maintainability. Consider refactoring this block to use intermediate variables and the .get() method for safer and clearer access to nested data.
For example:
qual_data = ranking_data.get("qual")
if not qual_data:
return -1, 999
ranking_info = qual_data.get("ranking")
if not ranking_info:
return -1, 999
if ranking_info.get("rank") is None:
return -1, 999This approach simplifies data extraction and would make the subsequent assignments to team_ranking and total_teams cleaner and safer.
…ing data Simplifies nested dict access for readability and safety.
|
Also I am the original creator of this app on Tidbyt 😃 |
2 participants
Summary
"qual": nullwhen an event is active but qualification matches haven't started yetnot in), not whether the value wasNone, causing a crash on theinoperator== Nonechecks at each nesting level inget_team_ranking()so the app gracefully falls back to the default ranking messageAlso submitted to the upstream Tidbyt community repo: tidbyt/community#3196