Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1857 +/- ##
=======================================
Coverage 86.67% 86.67%
=======================================
Files 9 9
Lines 1141 1141
=======================================
Hits 989 989
Misses 152 152 ☔ View full report in Codecov by Sentry. |
📝 WalkthroughWalkthroughBulk update of Nuneaton & Bedworth bin-calendar keys and all associated bin-type date arrays moved to a 2025–2026 schedule; mapping keys were extensively renamed. Control flow, function signatures, and external request behavior remain unchanged. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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}
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/NuneatonBedworthBoroughCouncil.py (1)
54-59:⚠️ Potential issue | 🟠 Major
Exception(...)is constructed but never raised — errors are silently swallowed.On lines 55, 57, and 59,
Exception(...)creates an exception object but doesn'traiseit, so all three error paths silently fall through and return an emptydatadict. The same issue exists inget_bin_dataon lines 943 and 946.Proposed fix
elif len(matches) > 1: - Exception("Multiple street URLs found. Please refine your search.") + raise Exception("Multiple street URLs found. Please refine your search.") else: - Exception("Street URL not found.") + raise Exception("Street URL not found.") else: - Exception("Failed to retrieve search results.") + raise Exception("Failed to retrieve search results.")And in
get_bin_data:- Exception("Bin data Download link not found.") + raise Exception("Bin data Download link not found.") else: - Exception("Failed to retrieve bin data.") + raise Exception("Failed to retrieve bin data.")Based 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/NuneatonBedworthBoroughCouncil.py` around lines 54 - 59, The code constructs Exception objects without raising them, causing errors to be swallowed; update the error branches in the search-results handling (where matches is checked) to raise the exceptions (use raise Exception(...)) instead of simply constructing them, and make the same change inside get_bin_data (the error paths referenced around the get_bin_data function at the reported lines) so unexpected formats explicitly raise errors rather than returning empty data.
🧹 Nitpick comments (1)
uk_bin_collection/uk_bin_collection/councils/NuneatonBedworthBoroughCouncil.py (1)
82-938: Consider extracting the date data to a separate JSON file.~850 lines of hardcoded dates dominate this module and will need a full replacement every year. Moving the data to a separate
.jsonfile (loaded at runtime) would keep the logic readable and make future annual updates a data-only change, easier to diff and review.Understandably low priority given the council's PDF-based workflow.
🤖 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/NuneatonBedworthBoroughCouncil.py` around lines 82 - 938, The module currently embeds ~850 lines of hardcoded dates in the bin_data dict; extract that payload into a separate JSON file (e.g., nuneaton_bedworth_bins.json) and update the module to load it at runtime and assign it to the existing bin_data symbol; modify the top-level variable or initializer that defines bin_data so it reads the JSON via json.load (use pathlib or package resource resolution to locate the file reliably) and keep the dict shape unchanged so existing references (bin_data, and any methods in the NuneatonBedworthBoroughCouncil class that use bin_data) continue to work; include a small fallback/error message if the JSON cannot be loaded.
🤖 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/NuneatonBedworthBoroughCouncil.py`:
- Around line 738-765: The Green Bin date lists for the "thursday-b" and
"friday-b" schedules in NuneatonBedworthBoroughCouncil.py appear to have an
extra date (26 entries) likely due to copy/paste from the Brown Bin; verify the
two Green Bin arrays against the source PDF and, if they should skip the
mid‑January collection like the other Green Bin schedules, remove the incorrect
mid‑January date so each "Green Bin" list for thursday-b and friday-b contains
25 dates to match the pattern used by the other schedules.
---
Outside diff comments:
In
`@uk_bin_collection/uk_bin_collection/councils/NuneatonBedworthBoroughCouncil.py`:
- Around line 54-59: The code constructs Exception objects without raising them,
causing errors to be swallowed; update the error branches in the search-results
handling (where matches is checked) to raise the exceptions (use raise
Exception(...)) instead of simply constructing them, and make the same change
inside get_bin_data (the error paths referenced around the get_bin_data function
at the reported lines) so unexpected formats explicitly raise errors rather than
returning empty data.
---
Nitpick comments:
In
`@uk_bin_collection/uk_bin_collection/councils/NuneatonBedworthBoroughCouncil.py`:
- Around line 82-938: The module currently embeds ~850 lines of hardcoded dates
in the bin_data dict; extract that payload into a separate JSON file (e.g.,
nuneaton_bedworth_bins.json) and update the module to load it at runtime and
assign it to the existing bin_data symbol; modify the top-level variable or
initializer that defines bin_data so it reads the JSON via json.load (use
pathlib or package resource resolution to locate the file reliably) and keep the
dict shape unchanged so existing references (bin_data, and any methods in the
NuneatonBedworthBoroughCouncil class that use bin_data) continue to work;
include a small fallback/error message if the JSON cannot be loaded.
| "Green Bin": [ | ||
| "2024-10-02", | ||
| "2024-10-16", | ||
| "2024-10-30", | ||
| "2024-11-13", | ||
| "2024-11-27", | ||
| "2024-12-11", | ||
| "2024-12-25", | ||
| "2025-01-08", | ||
| "2025-02-05", | ||
| "2025-02-19", | ||
| "2025-03-05", | ||
| "2025-03-19", | ||
| "2025-04-02", | ||
| "2025-04-16", | ||
| "2025-04-30", | ||
| "2025-05-14", | ||
| "2025-05-28", | ||
| "2025-06-11", | ||
| "2025-06-25", | ||
| "2025-07-09", | ||
| "2025-07-23", | ||
| "2025-08-06", | ||
| "2025-08-20", | ||
| "2025-09-03", | ||
| "2025-09-17", | ||
| "2025-10-09", | ||
| "2025-10-23", | ||
| "2025-11-06", | ||
| "2025-11-20", | ||
| "2025-12-04", | ||
| "2025-12-18", | ||
| "2026-01-01", | ||
| "2026-01-15", | ||
| "2026-01-29", | ||
| "2026-02-12", | ||
| "2026-02-26", | ||
| "2026-03-12", | ||
| "2026-03-26", | ||
| "2026-04-09", | ||
| "2026-04-23", | ||
| "2026-05-07", | ||
| "2026-05-21", | ||
| "2026-06-04", | ||
| "2026-06-18", | ||
| "2026-07-02", | ||
| "2026-07-16", | ||
| "2026-07-30", | ||
| "2026-08-13", | ||
| "2026-08-27", | ||
| "2026-09-10", | ||
| "2026-09-24", | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Extract and analyze bin_data from NuneatonBedworthBoroughCouncil.py
python3 << 'EOF'
import re
with open('uk_bin_collection/uk_bin_collection/councils/NuneatonBedworthBoroughCouncil.py', 'r') as f:
content = f.read()
# Find the bin_data dictionary assignment
match = re.search(r'bin_data\s*=\s*\{', content)
if not match:
print("ERROR: bin_data not found")
exit(1)
start_pos = match.start()
# Find the matching closing brace
depth = 0
pos = match.end() - 1
while pos < len(content):
if content[pos] == '{':
depth += 1
elif content[pos] == '}':
depth -= 1
if depth == 0:
break
pos += 1
bin_data_str = content[start_pos + len('bin_data = '):pos + 1]
# Parse the dictionary manually by extracting calendar keys and their bin types
lines = bin_data_str.split('\n')
current_cal = None
current_bin = None
data = {}
for line in lines:
line = line.strip()
# Detect calendar key (e.g., "monday-a":, "thursday-b":)
cal_match = re.match(r'"([^"]+)"\s*:\s*\{', line)
if cal_match:
current_cal = cal_match.group(1)
data[current_cal] = {}
continue
# Detect bin type with opening bracket (e.g., '"Brown Bin": [')
bin_match = re.match(r'"([^"]+)"\s*:\s*\[', line)
if bin_match and current_cal:
current_bin = bin_match.group(1)
data[current_cal][current_bin] = 0
continue
# Count date strings in the current bin (format: "YYYY-MM-DD")
if current_bin and current_cal:
date_count = len(re.findall(r'"(\d{4}-\d{2}-\d{2})"', line))
if date_count > 0:
data[current_cal][current_bin] += date_count
# Print results
print("Date counts per calendar and bin type:\n")
for cal_key in sorted(data.keys()):
bins = data[cal_key]
print(f"{cal_key}:")
for bin_type in sorted(bins.keys()):
print(f" {bin_type}: {bins[bin_type]}")
# Check consistency
brown_count = bins.get("Brown Bin", 0)
green_count = bins.get("Green Bin", 0)
if brown_count != 0 and green_count != 0:
diff = brown_count - green_count
print(f" → Difference (Brown - Green): {diff}")
print()
EOFRepository: robbrad/UKBinCollectionData
Length of output: 1182
thursday-b and friday-b Green Bin schedules break the pattern — possible copy-paste error.
In every other calendar (monday-a/b, tuesday-a/b, thursday-a, friday-a), the Green Bin list has 25 dates — one fewer than Brown Bin, with a mid-January collection omitted (presumably no garden waste in winter). However, thursday-b and friday-b Green Bin have 26 dates, identical to their Brown Bin lists.
Could you double-check these two against the source PDF? If Green Bin does skip a January collection for those schedules too, then one date should be removed from each.
🤖 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/NuneatonBedworthBoroughCouncil.py`
around lines 738 - 765, The Green Bin date lists for the "thursday-b" and
"friday-b" schedules in NuneatonBedworthBoroughCouncil.py appear to have an
extra date (26 entries) likely due to copy/paste from the Brown Bin; verify the
two Green Bin arrays against the source PDF and, if they should skip the
mid‑January collection like the other Green Bin schedules, remove the incorrect
mid‑January date so each "Green Bin" list for thursday-b and friday-b contains
25 dates to match the pattern used by the other schedules.
1 participant
Nuneaton and Bedworth Council still use PDFs which are difficult to obtain the data from. I've updated the hardcoded dates for this year's cycle.
Summary by CodeRabbit