Dec release#1757
Conversation
Docstrings generation was requested by @robbrad. * #1754 (comment) The following files were modified: * `uk_bin_collection/uk_bin_collection/councils/ArgyllandButeCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/ArmaghBanbridgeCraigavonCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/BirminghamCityCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/BlackpoolCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/ChelmsfordCityCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/EdinburghCityCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/FarehamBoroughCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/FifeCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/HaltonBoroughCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/HarlowCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/IsleOfAngleseyCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/KingsLynnandWestNorfolkBC.py` * `uk_bin_collection/uk_bin_collection/councils/LondonBoroughLambeth.py` * `uk_bin_collection/uk_bin_collection/councils/MidSussexDistrictCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/NorthHertfordshireDistrictCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/NorthumberlandCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/NorwichCityCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/RushmoorCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/SouthGloucestershireCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/SouthHollandDistrictCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/SouthLanarkshireCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/SouthamptonCityCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/ThurrockCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/WestOxfordshireDistrictCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/WiltshireCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/WinchesterCityCouncil.py`
📝 Add docstrings to `dec_release`
fix:Rushcliffe BC broken scraping
|
Warning Rate limit exceeded@robbrad has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 18 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (3)
WalkthroughThis pull request adds comprehensive docstrings to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
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 |
❌ 6 Tests Failed:
View the full list of 6 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
There was a problem hiding this comment.
Actionable comments posted: 3
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/HarlowCouncil.py (1)
1-3: Remove duplicate import.
requestsis imported twice (lines 1 and 3).-import requests - import requests from bs4 import BeautifulSoup
🧹 Nitpick comments (12)
uk_bin_collection/uk_bin_collection/councils/ThurrockCouncil.py (1)
45-46: Missing input validation could cause confusing errors.If
collection_dayis not indays_of_weekorroundis not inround_week,list.index()will raise aValueErrorwith a generic message. Consider validating inputs early with descriptive errors, consistent with the pattern used in other councils (e.g.,check_postcode,check_uprn).+ if collection_day not in days_of_week: + raise ValueError(f"Invalid collection day: {collection_day}. Expected one of {days_of_week}") + if round not in round_week: + raise ValueError(f"Invalid round: {round}. Expected 'Round A' or 'Round B'") offset_days = days_of_week.index(collection_day) round_collection = round_week.index(round)uk_bin_collection/uk_bin_collection/councils/HarlowCouncil.py (1)
52-53: PotentialAttributeErrorifsummaryis not found.If the page structure changes and no
div.summaryis found,summarywill beNoneandsummary.find_all()will raise anAttributeError. Consider adding a guard or raising a descriptive error.summary = soup.find("div", {"class": "summary"}) + if not summary: + raise ValueError("Could not find collection summary on page") collectionrows = summary.find_all("div", {"class": "collectionsrow"})uk_bin_collection/uk_bin_collection/councils/KingsLynnandWestNorfolkBC.py (1)
47-47:soup.prettify()result is unused.
prettify()returns a formatted string but doesn't modifysoupin place. This line has no effect and can be removed.soup = BeautifulSoup(response.content, features="html.parser") - soup.prettify()uk_bin_collection/uk_bin_collection/councils/NorthumberlandCouncil.py (1)
1-3:datetimemodule is shadowed by the class import.Line 1 imports the
datetimemodule, but line 3 importsdatetimeclass which shadows the module. The module import on line 1 becomes unreachable. If only the class is needed, remove line 1.-import datetime import time from datetime import datetimeuk_bin_collection/uk_bin_collection/councils/ArmaghBanbridgeCraigavonCouncil.py (1)
18-31: Docstring accurately documents the happy path.The docstring clearly describes parameters and return structure. However, consider updating the "Raises" section or adding a note that the function returns an empty
binslist (rather than raising) when the HTTP request fails (line 96 only prints an error message).Returns: dict: Dictionary with a "bins" key mapping to a list of collections. Each collection is a dict with: - "collectionDate" (str): Date in "DD/MM/YYYY" format. - "type" (str): One of "Domestic", "Recycling", or "Garden". The list is sorted in ascending order by the parsed collection date (format "%d/%m/%Y"). + + Note: + Returns an empty bins list if the HTTP request fails (non-200 status code). """uk_bin_collection/uk_bin_collection/councils/SouthGloucestershireCouncil.py (1)
7-20: Consider removing debug print statements.Line 10 contains
print(servicename)which appears to be debug output. Similarly, line 63 hasprint(collection). Consider removing these or converting to proper logging to avoid noise in production.def format_bin_data(key: str, date: datetime): formatted_date = date.strftime(date_format) servicename = key.get("hso_servicename") - print(servicename) if re.match(r"^Recycl", servicename) is not None:And in
parse_data:for collection in collection_data: - print(collection) item = collection.get('hso_nextcollection')uk_bin_collection/uk_bin_collection/councils/EdinburghCityCouncil.py (1)
58-65: Consider externalizing hardcoded start dates.The collection start dates are hardcoded to November 2025. While functional now, these will need periodic updates to maintain accuracy as collection schedules evolve. Consider storing these dates in a configuration file or database to simplify future maintenance.
uk_bin_collection/uk_bin_collection/councils/RushcliffeBoroughCouncil.py (1)
7-7: Consider explicit imports for clarity and static analysis compatibility.The static analysis tool flags
datetimeanddate_formatas potentially undefined due to the star import. While these are provided by thecommonmodule and work at runtime, explicit imports (as done inNorthHertfordshireDistrictCouncil.py) improve code clarity and IDE support.-from uk_bin_collection.uk_bin_collection.common import * +from datetime import datetime +from uk_bin_collection.uk_bin_collection.common import ( + check_postcode, + check_uprn, + create_webdriver, + date_format, +)uk_bin_collection/uk_bin_collection/councils/RushmoorCouncil.py (1)
18-34: Docstring accurately describes flow; consider expanding theRaisessectionThe implementation can raise a
ValueErrorboth whencheck_uprn(user_uprn)fails and when no collections are found. To keep the docstring aligned with behavior, consider mentioning that invalid UPRNs will also result in aValueError, not just the “no collections found” case.uk_bin_collection/uk_bin_collection/councils/FarehamBoroughCouncil.py (1)
16-31: Docstring looks good; minor clarity improvements for errors and date formatThe function can also raise a
ValueErrorfromcheck_postcode(user_postcode)when the postcode is invalid, not only when it is “not found on the website”. If you want the docstring to be fully explicit, you could mention this validation error separately. Likewise, sincecollectionDateis formatted usingdate_format, you might phrase the return description as “formatted according todate_format(currentlyDD/MM/YYYY)" so it stays correct if the global format ever changes.uk_bin_collection/uk_bin_collection/councils/ArgyllandButeCouncil.py (1)
21-36: Clear docstring; consider documenting validation and failure modesThe parameter and return descriptions match the implementation well, including UPRN normalization and
date_formatusage. If you want parity with other councils in this PR, you could add a briefRaisessection noting that invaliduprn/postcodewill causecheck_uprn/check_postcodeto raiseValueError, and that any Selenium/navigation issues will propagate as exceptions.uk_bin_collection/uk_bin_collection/councils/BirminghamCityCouncil.py (1)
67-84: Tighten parameter typing/validation to match real usage and messagesTwo small inconsistencies worth addressing:
pageis annotated and documented asstr, but it’s passed directly toget_token(page), which accessespage.text. That impliespageis a response-like object, not a plain string. Consider updating the type hint and docstring (e.g., “response object with a.textattribute”) to match actual usage.- The error messages and docstring say
uprn/postcode“must be a non-empty string”, but the current checks only guard againstNone. Empty strings (or whitespace-only values) will fall through tocheck_uprn/check_postcodeand fail there instead. Either strengthen the guards (for example, checkingif not uprn/if not postcode) or relax the wording in the docstring/messages so it only claims they are required, not necessarily already validated for non-emptiness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
uk_bin_collection/uk_bin_collection/councils/ArgyllandButeCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/ArmaghBanbridgeCraigavonCouncil.py(3 hunks)uk_bin_collection/uk_bin_collection/councils/BirminghamCityCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/BlackpoolCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/ChelmsfordCityCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/EdinburghCityCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/FarehamBoroughCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/FifeCouncil.py(3 hunks)uk_bin_collection/uk_bin_collection/councils/HaltonBoroughCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/HarlowCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/IsleOfAngleseyCouncil.py(6 hunks)uk_bin_collection/uk_bin_collection/councils/KingsLynnandWestNorfolkBC.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/LondonBoroughLambeth.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/MidSussexDistrictCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/NorthHertfordshireDistrictCouncil.py(4 hunks)uk_bin_collection/uk_bin_collection/councils/NorthumberlandCouncil.py(3 hunks)uk_bin_collection/uk_bin_collection/councils/NorwichCityCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/RushcliffeBoroughCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/RushmoorCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/SouthGloucestershireCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/SouthHollandDistrictCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/SouthLanarkshireCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/SouthamptonCityCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/ThurrockCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/WestOxfordshireDistrictCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/WiltshireCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/WinchesterCityCouncil.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.14.7)
uk_bin_collection/uk_bin_collection/councils/NorthumberlandCouncil.py
25-25: Docstring contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF002)
uk_bin_collection/uk_bin_collection/councils/RushcliffeBoroughCouncil.py
90-90: datetime may be undefined, or defined from star imports
(F405)
94-94: date_format may be undefined, or defined from star imports
(F405)
97-97: datetime may be undefined, or defined from star imports
(F405)
101-101: date_format may be undefined, or defined from star imports
(F405)
106-106: datetime may be undefined, or defined from star imports
(F405)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Unit Tests (3.12, 1.8.4)
- GitHub Check: Run Integration Tests (3.12, 1.8.4)
🔇 Additional comments (26)
uk_bin_collection/uk_bin_collection/councils/ThurrockCouncil.py (1)
15-27: Docstring accurately documents the API.The docstring clearly describes the unconventional parameter usage (paon for weekday, postcode for round) which is helpful for maintainers.
uk_bin_collection/uk_bin_collection/councils/FifeCouncil.py (2)
22-39: Comprehensive docstring with accurate parameter and return documentation.The docstring properly documents the parameters including optional ones, return structure, and potential exceptions. This aligns well with the implementation.
93-100: Helper function docstring is clear and useful.Documents the matching logic and return behavior appropriately.
uk_bin_collection/uk_bin_collection/councils/NorthumberlandCouncil.py (1)
41-56: Docstring accurately documents the method behavior.Parameters and return structure are well-documented, including the UPRN padding behavior.
uk_bin_collection/uk_bin_collection/councils/BlackpoolCouncil.py (1)
19-35: Well-documented docstring addition.The docstring accurately describes the parameters (
uprn,postcode), return structure, and theHTTPErrorthat can be raised by theraise_for_status()calls. This improves code maintainability without changing behavior.uk_bin_collection/uk_bin_collection/councils/ArmaghBanbridgeCraigavonCouncil.py (1)
42-53: Good documentation of internal helper.The docstring for
extract_bin_scheduleclearly explains its purpose and parameters, making the code more maintainable.uk_bin_collection/uk_bin_collection/councils/WestOxfordshireDistrictCouncil.py (1)
25-39: Comprehensive docstring added.The docstring clearly documents all relevant kwargs including
paon,postcode,web_driver, andheadless, along with the return structure. This improves discoverability and maintainability.uk_bin_collection/uk_bin_collection/councils/NorwichCityCouncil.py (1)
18-34: Thorough docstring with accurate exception documentation.The docstring correctly documents all three
Exceptionraise points (initial page load failure, address not found, no scheduled services), which aligns with the actual implementation at lines 55, 75, and 87. Good attention to detail.uk_bin_collection/uk_bin_collection/councils/SouthGloucestershireCouncil.py (1)
25-39: Accurate docstring documenting the ValueError.The docstring correctly documents the
ValueErrorraised when no collection data is found (line 54). The parameter and return structure documentation is clear and helpful.uk_bin_collection/uk_bin_collection/councils/MidSussexDistrictCouncil.py (1)
20-36: Excellent documentation addition.The docstring comprehensively describes the method's purpose, parameters, return structure, and error conditions. The documentation accurately reflects the implementation.
uk_bin_collection/uk_bin_collection/councils/WiltshireCouncil.py (1)
17-34: Clear and comprehensive docstring.The documentation accurately describes the multi-month query logic, input parameters, return structure, and error handling. Well done.
uk_bin_collection/uk_bin_collection/councils/LondonBoroughLambeth.py (1)
18-34: Well-documented API interaction.The docstring clearly describes the Lambeth API endpoint, parameter requirements, commercial container normalization logic, and return structure.
uk_bin_collection/uk_bin_collection/councils/SouthHollandDistrictCouncil.py (1)
16-30: Thorough documentation of API integration.The docstring accurately describes the JSON-RPC endpoint interaction, parameter requirements, return structure, and sorting behavior.
uk_bin_collection/uk_bin_collection/councils/HaltonBoroughCouncil.py (1)
25-40: Comprehensive documentation of browser-based scraping.The docstring clearly describes the Selenium-based approach, input parameters including webdriver configuration, and the returned data structure.
uk_bin_collection/uk_bin_collection/councils/SouthLanarkshireCouncil.py (1)
19-29: Clear documentation of HTML parsing logic.The docstring accurately describes the page parsing approach, input requirements, and return structure.
uk_bin_collection/uk_bin_collection/councils/EdinburghCityCouncil.py (1)
25-38: Well-documented scheduling algorithm.The docstring clearly explains the fortnightly rota logic, input parameter semantics, and return structure.
uk_bin_collection/uk_bin_collection/councils/WinchesterCityCouncil.py (1)
20-31: Improved docstring clarity.The wording refinements make the documentation more concise while maintaining accuracy.
uk_bin_collection/uk_bin_collection/councils/SouthamptonCityCouncil.py (1)
19-34: LGTM! Docstring accurately documents the function behavior.The docstring correctly describes the return structure, date format, and error conditions (ValueError for missing calendar view, HTTPError for request failures) which match the implementation at lines 67 and 74-76.
uk_bin_collection/uk_bin_collection/councils/ChelmsfordCityCouncil.py (1)
18-33: LGTM! Docstring correctly documents error conditions.The expanded documentation accurately reflects the two ValueError scenarios: when no collection round is found (line 111) and when the ICS calendar link cannot be located (line 106).
uk_bin_collection/uk_bin_collection/councils/RushcliffeBoroughCouncil.py (2)
84-109: Good implementation for capturing multiple collection dates.The updated regex correctly captures both collection dates, and the logic properly creates separate bin entries for each. The sorting ensures consistent output ordering.
One consideration: the regex expects exactly two dates with "and" between them. If the council page ever shows a single date or different format, no results will be captured. This is acceptable given the scraper's nature, but worth noting for future maintenance.
110-118: Good resource cleanup pattern.The try/except/finally structure ensures the WebDriver is properly closed even when exceptions occur, preventing resource leaks. This aligns with the pattern used in other council implementations.
uk_bin_collection/uk_bin_collection/councils/NorthHertfordshireDistrictCouncil.py (2)
34-58: Well-documented with thorough input validation.The enhanced docstring clearly describes the function's purpose, parameters, return value, and comprehensive error conditions. The input normalization (strip/lowercase) before matching is a good defensive practice.
160-179: Clear documentation of the dual-input pattern.The docstring effectively communicates that users can provide either a UPRN directly or a postcode+paon combination, with clear error conditions documented. This matches the implementation logic at lines 184-197.
uk_bin_collection/uk_bin_collection/councils/IsleOfAngleseyCouncil.py (3)
27-35: LGTM! Clear initialization documentation.The docstring accurately describes the session setup with
self._sessionand theself._have_sessionflag pattern for lazy authentication.
135-152: Well-documented dual-input interface.The docstring clearly explains that users can provide either
uprnalone orpostcodewithpaon/number, matching the implementation at lines 155-170.
219-226: Year inference logic handles edge cases well.The approach of assuming current year and rolling over to next year if the date is more than 30 days in the past is a reasonable heuristic for handling date strings without year information. The 30-day buffer prevents false rollovers for recently-passed dates.
| Parameters: | ||
| uprn (str): Unique Property Reference Number provided via kwargs key "uprn". The value will be validated and left-padded with zeros to 12 characters before use. |
There was a problem hiding this comment.
Docstring mentions parameters not used by this method.
The docstring references web_driver and headless parameters, but this council uses requests (not Selenium) and doesn't accept or use these kwargs. Remove these from the docstring to avoid confusion.
Parameters:
uprn (str): Unique Property Reference Number provided via kwargs key "uprn". The value will be validated and left-padded with zeros to 12 characters before use.
-
- web_driver (str, optional, in kwargs): WebDriver backend identifier passed to the webdriver factory.
- headless (bool, optional, in kwargs): Whether to run the browser in headless mode.Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In uk_bin_collection/uk_bin_collection/councils/KingsLynnandWestNorfolkBC.py
around lines 23-24, the docstring incorrectly lists web_driver and headless
parameters that are not accepted or used (this module uses requests, not
Selenium); remove those parameters from the Parameters section so it only
documents the actual kwargs used (e.g., uprn) and update wording to match the
real behavior (validate/pad uprn to 12 chars) to avoid confusion.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…ncil.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary by CodeRabbit
Release Notes
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.