fix: Update collection for Westmorland and Furness#2004
fix: Update collection for Westmorland and Furness#2004patch0 wants to merge 3 commits intorobbrad:masterfrom
Conversation
The `colour` field is no longer populated, so use `type` instead. Also the calendar shows a whole 12 month period, so update script to cope.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe WestMorlandAndFurness council parser is updated to extract bin type from the ChangesWestMorlandAndFurness Parser Update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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. Review rate limit: 0/1 reviews remaining, refill in 51 minutes and 1 second.Comment |
patch0
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/WestMorlandAndFurness.py`:
- Around line 46-48: The code calls collectionday.find("span", {"class":
"waste-collection__day--type"}).text.strip() without checking for None; add a
null guard in the parsing routine (the block that sets bintype from
collectionday) to detect when find(...) returns None and raise a clear exception
(e.g., ValueError) that includes context (the collectionday element or a short
HTML snippet and the missing class name) instead of letting an AttributeError
occur; update the logic around the bintype assignment in
WestMorlandAndFurness.py to perform the find, check for None, and then call
.text.strip() only when present.
🪄 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: a627aff1-b6aa-4bf0-a8d0-ca180fdbf700
📒 Files selected for processing (1)
uk_bin_collection/uk_bin_collection/councils/WestMorlandAndFurness.py
| bintype = collectionday.find( | ||
| "span", {"class": "waste-collection__day--colour"} | ||
| ).text | ||
| "span", {"class": "waste-collection__day--type"} | ||
| ).text.strip() |
There was a problem hiding this comment.
Add a null guard before calling .text.strip() on the .find() result.
If the waste-collection__day--type span is absent (e.g., due to a future feed change), .find() returns None and the chained .text.strip() raises a bare AttributeError with no diagnostic context.
🛡️ Proposed fix for null safety
- bintype = collectionday.find(
- "span", {"class": "waste-collection__day--type"}
- ).text.strip()
+ type_span = collectionday.find(
+ "span", {"class": "waste-collection__day--type"}
+ )
+ if type_span is None:
+ raise ValueError(
+ f"Could not find bin type span for day {day} in month {collectionmonth}"
+ )
+ bintype = type_span.text.strip()Based on learnings: in uk_bin_collection/**/*.py, prefer explicit failures (raise exceptions on unexpected formats) over silent defaults or swallowed errors, using clear exception types with documented error causes.
📝 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.
| bintype = collectionday.find( | |
| "span", {"class": "waste-collection__day--colour"} | |
| ).text | |
| "span", {"class": "waste-collection__day--type"} | |
| ).text.strip() | |
| type_span = collectionday.find( | |
| "span", {"class": "waste-collection__day--type"} | |
| ) | |
| if type_span is None: | |
| raise ValueError( | |
| f"Could not find bin type span for day {day} in month {collectionmonth}" | |
| ) | |
| bintype = type_span.text.strip() |
🤖 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/WestMorlandAndFurness.py` around
lines 46 - 48, The code calls collectionday.find("span", {"class":
"waste-collection__day--type"}).text.strip() without checking for None; add a
null guard in the parsing routine (the block that sets bintype from
collectionday) to detect when find(...) returns None and raise a clear exception
(e.g., ValueError) that includes context (the collectionday element or a short
HTML snippet and the missing class name) instead of letting an AttributeError
occur; update the logic around the bintype assignment in
WestMorlandAndFurness.py to perform the find, check for None, and then call
.text.strip() only when present.
There was a problem hiding this comment.
This pattern is already in use in the rest of this parser, and I'm looking to avoid a wider refactor at this stage.
There was a problem hiding this comment.
@patch0, understood — the unguarded .find().text.strip() pattern is consistent with the rest of the parser (e.g. the .find("span", {"class": "waste-collection__day--day"}).text.strip() call a few lines above). Hardening only this one spot in isolation would be inconsistent. I'll drop the suggestion.
✏️ Learnings added
Learnt from: patch0
Repo: robbrad/UKBinCollectionData PR: 2004
File: uk_bin_collection/uk_bin_collection/councils/WestMorlandAndFurness.py:0-0
Timestamp: 2026-05-04T10:04:32.775Z
Learning: In `uk_bin_collection/uk_bin_collection/councils/WestMorlandAndFurness.py`, the existing parser intentionally uses unguarded `.find().text.strip()` calls (without None checks) throughout. Null-guard suggestions for individual `.find()` calls should not be raised in isolation; a wider refactor of the whole file would be needed to apply them consistently.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: naggie
Repo: robbrad/UKBinCollectionData PR: 1826
File: uk_bin_collection/uk_bin_collection/councils/HuntingdonDistrictCouncil.py:47-54
Timestamp: 2026-01-24T22:15:19.408Z
Learning: 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. This ensures format changes are detected early. Use clear exception types, document error causes, and add tests to verify that invalid inputs raise as expected.
There was a problem hiding this comment.
Pull request overview
Updates the Westmorland and Furness council scraper so this council can keep returning usable bin collection data after the site changed its schedule markup and expanded the visible calendar range.
Changes:
- Switch the parsed bin label from the now-empty
waste-collection__day--colourelement towaste-collection__day--type. - Update the year-assignment logic to handle a calendar that spans a full 12-month window.
- Keep the output sorted as standard
type/collectionDaterecords for the wider UK bin collection codebase.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # The calendar shows the next 12 months, so if the month steps back in | ||
| # time, assume it is for the following year. | ||
| if (collectiondate.month < current_month): | ||
| collectiondate = collectiondate.replace(year=(current_year + 1)) |
There was a problem hiding this comment.
While true, I think this is outside the scope of what I was trying to achieve here.
| bintype = collectionday.find( | ||
| "span", {"class": "waste-collection__day--colour"} | ||
| ).text | ||
| "span", {"class": "waste-collection__day--type"} | ||
| ).text.strip() |
There was a problem hiding this comment.
This is not true for this council, as far as I can tell.
This avoids having to check if `text` returns anything
Undoing last change, because the `text.strip()` pattern is already in this parser. Trying to keep things small and consistent. This reverts commit 3880277.
Closes #1965
The
colourfield is no longer populated, so usetypeinstead.Also the calendar shows a whole 12 month period, so update script to cope.
Before
{ "bins": [ { "type": "", "collectionDate": "01/01/2026" }, { "type": "", "collectionDate": "08/01/2026" }, { "type": "", "collectionDate": "15/01/2026" }, { "type": "", "collectionDate": "22/01/2026" }, { "type": "", "collectionDate": "29/01/2026" }, ... snip ... { "type": "", "collectionDate": "04/12/2026" }, { "type": "", "collectionDate": "11/12/2026" }, { "type": "", "collectionDate": "18/12/2026" }, { "type": "", "collectionDate": "25/12/2026" } ] }After
{ "bins": [ { "type": "General Waste", "collectionDate": "01/05/2026" }, { "type": "Recyclable waste", "collectionDate": "08/05/2026" }, { "type": "Garden waste", "collectionDate": "08/05/2026" }, { "type": "General Waste", "collectionDate": "15/05/2026" }, { "type": "Recyclable waste", "collectionDate": "22/05/2026" }, { "type": "Garden waste", "collectionDate": "22/05/2026" }, { "type": "General Waste", "collectionDate": "30/05/2026" }, ... snip ... { "type": "Recyclable waste", "collectionDate": "23/04/2027" }, { "type": "General Waste", "collectionDate": "30/04/2027" } ] }Summary by CodeRabbit