fix: GreatYarmouthBoroughCouncil - use current year instead of hardcoded 2025#1950
Open
InertiaUK wants to merge 1 commit intorobbrad:masterfrom
Open
fix: GreatYarmouthBoroughCouncil - use current year instead of hardcoded 2025#1950InertiaUK wants to merge 1 commit intorobbrad:masterfrom
InertiaUK wants to merge 1 commit intorobbrad:masterfrom
Conversation
Contributor
📝 WalkthroughWalkthroughUpdated the Great Yarmouth Borough Council scraper with a fixed user-agent string for WebDriver identification and replaced hardcoded "2025" year with dynamic current year detection in date parsing logic, ensuring future compatibility. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
uk_bin_collection/uk_bin_collection/councils/GreatYarmouthBoroughCouncil.py (1)
107-113:⚠️ Potential issue | 🟠 MajorDo not silently ignore date parse failures.
except ValueError: continuecan hide council format changes and return incomplete data without failing fast.Suggested fix (explicit failure with context)
- except ValueError: - continue + except ValueError as exc: + raise ValueError( + f"Unexpected collection date format: '{date_text}'" + ) from excBased on learnings: In
uk_bin_collection/**/*.py, when parsing council bin collection data, prefer explicit failures (raise exceptions on unexpected formats) over silent defaults or swallowed errors.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@uk_bin_collection/uk_bin_collection/councils/GreatYarmouthBoroughCouncil.py` around lines 107 - 113, The current try/except around datetime.strptime (the block handling date_text and date_obj) silently swallows ValueError which can hide format changes; replace the bare except that does "continue" with explicit error handling: catch the ValueError, enrich and re-raise it (or log and raise) with context including the offending date_text and the surrounding source identifier (e.g. the page/element id), so callers fail fast; update the code around date_text / datetime.strptime / date_obj to raise a new ValueError (or RuntimeError) that includes date_text and any relevant identifiers instead of silently continuing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@uk_bin_collection/uk_bin_collection/councils/GreatYarmouthBoroughCouncil.py`:
- Around line 109-111: The code builds date_obj using
datetime.strptime(date_text + " " + str(datetime.today().year)) and then skips
it if date_obj.date() < datetime.today().date(), which incorrectly drops
next-year January dates when run in late December; update the logic in
GreatYarmouthBoroughCouncil (where date_text and date_obj are created) to detect
year rollover before skipping by: after parsing with the current year, if
date_obj < today and date_obj.month < today.month (or specifically if
today.month == 12 and date_obj.month == 1) then add one year to date_obj (e.g.,
replace year with date_obj.year + 1) so that January dates in the next year are
preserved, and only then apply the past-date skip.
---
Outside diff comments:
In `@uk_bin_collection/uk_bin_collection/councils/GreatYarmouthBoroughCouncil.py`:
- Around line 107-113: The current try/except around datetime.strptime (the
block handling date_text and date_obj) silently swallows ValueError which can
hide format changes; replace the bare except that does "continue" with explicit
error handling: catch the ValueError, enrich and re-raise it (or log and raise)
with context including the offending date_text and the surrounding source
identifier (e.g. the page/element id), so callers fail fast; update the code
around date_text / datetime.strptime / date_obj to raise a new ValueError (or
RuntimeError) that includes date_text and any relevant identifiers instead of
silently continuing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5ec556d2-6c5d-47c1-a0dc-cf1e34100589
📒 Files selected for processing (1)
uk_bin_collection/uk_bin_collection/councils/GreatYarmouthBoroughCouncil.py
Comment on lines
+109
to
111
| date_obj = datetime.strptime(date_text + " " + str(datetime.today().year), "%A %d %B %Y") | ||
| if date_obj.date() < datetime.today().date(): | ||
| continue # Skip past dates |
Contributor
There was a problem hiding this comment.
Line 109: current-year parsing still drops next-year January dates around year-end.
When run in late December, January dates are parsed into the current year and skipped as past on Line 110.
Suggested fix (handle year rollover before skipping)
- date_obj = datetime.strptime(date_text + " " + str(datetime.today().year), "%A %d %B %Y")
- if date_obj.date() < datetime.today().date():
- continue # Skip past dates
+ today = datetime.today().date()
+ date_obj = datetime.strptime(
+ f"{date_text} {today.year}", "%A %d %B %Y"
+ )
+ if date_obj.date() < today:
+ rollover_obj = datetime.strptime(
+ f"{date_text} {today.year + 1}", "%A %d %B %Y"
+ )
+ if rollover_obj.date() >= today:
+ date_obj = rollover_obj
+ else:
+ continue # true past date📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| date_obj = datetime.strptime(date_text + " " + str(datetime.today().year), "%A %d %B %Y") | |
| if date_obj.date() < datetime.today().date(): | |
| continue # Skip past dates | |
| today = datetime.today().date() | |
| date_obj = datetime.strptime( | |
| f"{date_text} {today.year}", "%A %d %B %Y" | |
| ) | |
| if date_obj.date() < today: | |
| rollover_obj = datetime.strptime( | |
| f"{date_text} {today.year + 1}", "%A %d %B %Y" | |
| ) | |
| if rollover_obj.date() >= today: | |
| date_obj = rollover_obj | |
| else: | |
| continue # true past date |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@uk_bin_collection/uk_bin_collection/councils/GreatYarmouthBoroughCouncil.py`
around lines 109 - 111, The code builds date_obj using
datetime.strptime(date_text + " " + str(datetime.today().year)) and then skips
it if date_obj.date() < datetime.today().date(), which incorrectly drops
next-year January dates when run in late December; update the logic in
GreatYarmouthBoroughCouncil (where date_text and date_obj are created) to detect
year rollover before skipping by: after parsing with the current year, if
date_obj < today and date_obj.month < today.month (or specifically if
today.month == 12 and date_obj.month == 1) then add one year to date_obj (e.g.,
replace year with date_obj.year + 1) so that January dates in the next year are
preserved, and only then apply the past-date skip.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The date parser was appending a hardcoded " 2025" to parsed date strings, then skipping all dates as "past" since it's now 2026. Changed to use datetime.today().year so it works across year boundaries.
One-line fix in the collection date parsing loop.
Tested with UPRN 100090834754 in Gorleston — returns General Waste, Garden Waste, and Recycling dates.
Summary by CodeRabbit
Release Notes