tests/test_cinoh_city_council.py (1)

23-75: Add docstrings to test functions

Adding docstrings to test functions would improve test clarity and documentation.

Example for one test:

 def test_count():
+    """Verify the correct number of meetings are parsed from the test data."""
     assert len(parsed_items) == 21

city_scrapers/spiders/cinoh_city_council.py (2)

11-12: Remove unused link_types attribute

The link_types attribute is defined but is empty and doesn't appear to be used in the spider. If it's not required, consider removing it to clean up the code.

Apply this change to remove the unused attribute:

     # Add the titles of any links not included in the scraped results
-    link_types = []

---

15-20: Update docstring to reflect actual methods used

The docstring refers to _parse_title and _parse_start methods, which are not defined in this spider. Update the docstring to accurately reflect the implemented methods.

Apply this change to correct the docstring:

         """
-        `parse_legistar` should always `yield` Meeting items.
-
-        Change the `_parse_title`, `_parse_start`, etc methods to fit your scraping
-        needs.
+        Parses Legistar events and yields Meeting items.
+
+        Uses helper methods `_parse_classification`, `_parse_location`, and `_parse_links`
+        to extract meeting details.
         """

Configuration used: CodeRabbit UI
Review profile: CHILL

city_scrapers/spiders/cinoh_city_council.py (3)

20-32: ⚠️ Potential issue

Add error handling for missing keys in meeting data.

Direct dictionary access without safety checks could raise KeyError exceptions.

Apply these changes to safely handle missing data:

             meeting = Meeting(
-                title=obj["Name"]["label"],
+                title=obj.get("Name", {}).get("label", ""),
                 description="",
                 classification=self._parse_classification(obj),
                 start=self.legistar_start(obj),
                 end=None,
                 all_day=False,
                 time_notes="",
                 status=self._parse_status(obj),
                 location=self._parse_location(obj),
                 links=self._parse_links(obj),
-                source=self.legistar_source(obj),
+                source=self.legistar_source(obj) or "",
             )

---

50-55: ⚠️ Potential issue

Add error handling for location parsing.

Direct access to obj["Meeting Location"] could raise KeyError.

     def _parse_location(self, obj):
-        room = obj["Meeting Location"]
+        room = obj.get("Meeting Location", "Location not provided")
         return {
             "address": "801 Plum St. Cincinnati, OH 45202",
-            "name": f"{room}"
+            "name": room
         }

---

57-74: ⚠️ Potential issue

Fix link parsing to include all available links.

The current implementation using separate if statements with return only captures the first matching link. This means potentially important links are being missed.

     def _parse_links(self, obj):
-        
-        if obj.get("Name"):
-            return [{"title": "meeting page", "href": obj["Name"]["url"]}]
-        if obj.get("iCalendar"):
-            return [{"title": "iCalendar", "href": obj["iCalendar"]["url"]}]
-        if obj.get("Meeting Details"):
-            return [{"title": "Meeting Details", "href": obj["Meeting Details"]["url"]}]
-        if obj.get("Agenda"):
-            return [{"title": "Agenda", "href": obj["Agenda"]["url"]}]
-        if obj.get("Agenda Packet"):
-            return [{"title": "Agenda Packet", "href": obj["Agenda Packet"]["url"]}]
-        if obj.get("Minutes"):
-            return [{"title": "Minutes", "href": obj["Minutes"]["url"]}]
-        if obj.get("Video"):
-            return [{"title": "Video", "href": obj["Video"]["url"]}]
-        
-
+        links = []
+        if obj.get("Name", {}).get("url"):
+            links.append({"title": "Meeting Page", "href": obj["Name"]["url"]})
+        if obj.get("iCalendar", {}).get("url"):
+            links.append({"title": "iCalendar", "href": obj["iCalendar"]["url"]})
+        if obj.get("Meeting Details", {}).get("url"):
+            links.append({"title": "Meeting Details", "href": obj["Meeting Details"]["url"]})
+        if obj.get("Agenda", {}).get("url"):
+            links.append({"title": "Agenda", "href": obj["Agenda"]["url"]})
+        if obj.get("Agenda Packet", {}).get("url"):
+            links.append({"title": "Agenda Packet", "href": obj["Agenda Packet"]["url"]})
+        if obj.get("Minutes", {}).get("url"):
+            links.append({"title": "Minutes", "href": obj["Minutes"]["url"]})
+        if obj.get("Video", {}).get("url"):
+            links.append({"title": "Video", "href": obj["Video"]["url"]})
+        return links

city_scrapers/spiders/cinoh_city_council.py (1)

13-18: Improve docstring clarity and fix typos.

The docstring could be more informative and contains typos:

• "calander" should be "calendar"
• Add information about the return type and parameters

Update the docstring to:

-        """
-        Parse upcoming and past meetings from the Cincinnati City Council meetings table.
-
-        Oftentimes, the columns: meeting details, agenda, minutes, and video are left blank on the calander
-        but when they are, they are in the form of links. 
-        """
+        """Parse upcoming and past meetings from the Cincinnati City Council meetings table.
+
+        Args:
+            response: JSON response from Legistar API containing meeting data
+
+        Yields:
+            Meeting: Scraped meeting items with details including title, time, location, and links.
+            
+        Note:
+            The columns (meeting details, agenda, minutes, and video) are sometimes empty on the calendar,
+            but when present, they contain links to the respective resources.
+        """

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

city_scrapers/spiders/cinoh_city_council.py (1)

22-34: ⚠️ Potential issue

Add error handling for missing keys in meeting data.

Direct dictionary access without checking for key existence could raise KeyError exceptions.

Apply these changes to safely handle missing keys:

             meeting = Meeting(
-                title=obj["Name"]["label"],
+                title=obj.get("Name", {}).get("label", "No title provided"),
                 description="",
                 classification=self._parse_classification(obj),
                 start=self.legistar_start(obj),
                 end=None,
                 all_day=False,
                 time_notes="",
                 status=self._parse_status(obj),
                 location=self._parse_location(obj),
                 links=self._parse_links(obj),
-                source=self.legistar_source(obj),
+                source=self.legistar_source(obj) if obj else "",
             )

city_scrapers/spiders/cinoh_city_council.py (1)

14-20: Enhance method documentation.

The docstring could be more descriptive about the method's parameters, return value, and specific behavior.

Apply this change:

     def parse_legistar(self, response):
         """
-        Parse upcoming and past meetings from the Cincinnati City Council meetings table.
-
-        Oftentimes, the columns: meeting details, agenda, minutes, and video are left blank on the calander
-        but when they are, they are in the form of links.
+        Parse meeting information from the Legistar API response.
+
+        Args:
+            response: JSON response from Legistar API containing meeting data
+
+        Yields:
+            Meeting: Scraped meeting items with details including title, date, location,
+                    status, and associated document links
+
+        Note:
+            Meeting documents (agenda, minutes, video) may be unavailable or present as links
+            in the calendar view.
         """

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

city_scrapers/spiders/cinoh_city_council.py (4)

25-37: ⚠️ Potential issue

Add error handling for missing keys.

The current implementation could raise KeyError exceptions if the response data is malformed.

Apply this change to handle missing keys safely:

             meeting = Meeting(
-                title=obj["Name"]["label"],
+                title=obj.get("Name", {}).get("label", "No title provided"),
                 description="",
                 classification=self._parse_classification(obj),
                 start=self.legistar_start(obj),
                 end=None,
                 all_day=False,
                 time_notes="",
                 status=self._parse_status(obj),
                 location=self._parse_location(obj),
                 links=self._parse_links(obj),
                 source=self.legistar_source(obj),
             )

---

49-63: 🛠️ Refactor suggestion

Enhance cancellation detection logic.

The current implementation:

1. Has unsafe key access
2. Uses a very specific cancellation notice format
3. Could miss other cancellation indicators

     def _parse_status(self, obj):
         date = obj["Meeting Date"]
         parsed_date = parse(date, fuzzy=True)
 
-        if (
-            obj["Meeting Location"]
-            == "Council Chambers, Room 300 NOTICE OF CANCELLATION"
-        ):
+        location = obj.get("Meeting Location", "").lower()
+        if "notice of cancellation" in location or "canceled" in location:
             return "cancelled"
         elif parsed_date < datetime.today():
             return "passed"
         else:
             return "tentative"

---

64-66: ⚠️ Potential issue

Add error handling for missing location.

The method could raise KeyError if the "Meeting Location" key is missing.

     def _parse_location(self, obj):
-        room = obj["Meeting Location"]
+        room = obj.get("Meeting Location", "No room provided")
         return {"address": "801 Plum St. Cincinnati, OH 45202", "name": f"{room}"}

---

68-97: 🛠️ Refactor suggestion

Refactor link parsing for better maintainability and reliability.

Current implementation has several issues:

1. Unsafe dictionary access
2. Inefficient comparison operators
3. Repetitive code structure

     def _parse_links(self, obj):
         links = []
-
-        if obj.get("Name"):
-            links.append({"title": "meeting page", "href": obj["Name"]["url"]})
-
-        if obj.get("iCalendar"):
-            links.append({"title": "iCalendar", "href": obj["iCalendar"]["url"]})
-
-        if not obj.get("Meeting Details") == "Meeting\u00a0details":
-            links.append(
-                {"title": "Meeting Details", "href": obj["Meeting Details"]["url"]}
-            )
-
-        if not obj.get("Agenda") == "Not\u00a0available":
-            links.append({"title": "Agenda", "href": obj["Agenda"]["url"]})
-
-        if not obj.get("Agenda Packet") == "Not\u00a0available":
-            links.append(
-                {"title": "Agenda Packet", "href": obj["Agenda Packet"]["url"]}
-            )
-
-        if not obj.get("Minutes") == "Not\u00a0available":
-            links.append({"title": "Minutes", "href": obj["Minutes"]["url"]})
-
-        if not obj.get("Video") == "Not\u00a0available":
-            links.append({"title": "Video", "href": obj["Video"]["url"]})
+        documents = {
+            "Name": "meeting page",
+            "iCalendar": "iCalendar",
+            "Meeting Details": "Meeting Details",
+            "Agenda": "Agenda",
+            "Agenda Packet": "Agenda Packet",
+            "Minutes": "Minutes",
+            "Video": "Video"
+        }
+
+        for key, title in documents.items():
+            doc = obj.get(key, {})
+            if isinstance(doc, dict) and doc.get("url"):
+                if key in ["Meeting Details"] and doc.get("label") == "Meeting\u00a0details":
+                    continue
+                if doc.get("label") != "Not\u00a0available":
+                    links.append({"title": title, "href": doc["url"]})
 
         return links

This refactoring:

1. Uses a dictionary to define document types
2. Handles missing keys safely
3. Properly checks for unavailable documents
4. Reduces code duplication

🧰 Tools

🪛 Ruff (0.8.2)

78-78: Use obj.get("Meeting Details") != "Meeting\xa0details" instead of not obj.get("Meeting Details") == "Meeting\xa0details"

Replace with != operator

(SIM201)

---

83-83: Use obj.get("Agenda") != "Not\xa0available" instead of not obj.get("Agenda") == "Not\xa0available"

Replace with != operator

(SIM201)

---

86-86: Use obj.get("Agenda Packet") != "Not\xa0available" instead of not obj.get("Agenda Packet") == "Not\xa0available"

Replace with != operator

(SIM201)

---

91-91: Use obj.get("Minutes") != "Not\xa0available" instead of not obj.get("Minutes") == "Not\xa0available"

Replace with != operator

(SIM201)

---

94-94: Use obj.get("Video") != "Not\xa0available" instead of not obj.get("Video") == "Not\xa0available"

Replace with != operator

(SIM201)

city_scrapers/spiders/cinoh_city_council.py (1)

16-23: Enhance docstring with more details.

Add information about:

• The method's parameters
• The return value (yielded Meeting objects)
• The structure of the response object

     def parse_legistar(self, response):
         """
-        Parse upcoming and past meetings from the
-        Cincinnati City Council meetings table.
+        Parse upcoming and past meetings from the Cincinnati City Council meetings table.
+
+        Args:
+            response: JSON response from Legistar API containing meeting data
+
+        Yields:
+            Meeting: Meeting object containing parsed meeting information
 
         Oftentimes, the columns: meeting details, agenda,
         minutes, and video are left blank on the calander
         but when they are, they are in the form of links.
         """

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro