-
Notifications
You must be signed in to change notification settings - Fork 171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
District teams list: only show teams who've competed that year #5289
base: py3
Are you sure you want to change the base?
Conversation
@@ -94,10 +95,13 @@ def __init__(self, district_key: DistrictKey) -> None: | |||
def _query_async( | |||
self, district_key: DistrictKey | |||
) -> Generator[Any, Any, List[Team]]: | |||
year = int(re.match(r"\d{4,}", district_key).group(0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming there's some sort of helper for parsing these but I couldn't find one.
Codecov Report
@@ Coverage Diff @@
## py3 #5289 +/- ##
===========================================
- Coverage 92.25% 71.22% -21.04%
===========================================
Files 640 8 -632
Lines 40277 344 -39933
Branches 60 60
===========================================
- Hits 37159 245 -36914
+ Misses 3110 91 -3019
Partials 8 8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this seems good. Though I wonder if filtering on query is the correct thing to do, or if we should be doing more validation on write?
cc @phil-lopreiato thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes! Can you fix the failing tests (and add a test that tests your changes)?
Description
The district teams list sometimes includes teams that didn't actually compete in a given year. This seems to be an issue with the FIRST API:
This change filters the list by only showing teams that have competed (or will compete) in an event that year.
Motivation and Context
Fixes #5106!
How Has This Been Tested?
I've verified that after this change, the NE team list matches the one on FIRST's website.
Screenshots (if appropriate):
Types of changes