Repository: hhursev/recipe-scrapers
Pull request #1031 (2024-03-18 18:19:46)

Refactor of _utils.py and associated test cases
**_utils.py**
- Expanded on `FRACTIONS` coverage and `TIME_REGEX` readability
- Expanded `_extract_fractional` coverage capabilities. Now handling a wider range of fractional representations specifically unicode fraction characters.
  - Now handles mixed numbers that include unicode fraction characters formatted like `1⅔` by iterating through the `FRACTIONS` dictionary to find matches & processing separately as needed.
  - Now iterates through `FRACTIONS` for a match to replace unicode characters
  - Conversion logic located within a try-except block to return a `ValueError` with a descriptive error message for unrecognized fraction inputs.
  - Improved handling of fractions with and without whole numbers present by checking for existence and processing separately (mixed numbers ('1 1/2') and simple fractions ('1/2')
  - Adjusted some general formatting for consistency throughout code base
- Updated `get_minutes` for readability and adjusted for new functionality from `_extract_fractional`
- Updated `url_path_to_dict`
    - Updated host re.pattern to exclude port / path delimiters.
- Updated `get_host_name` to use .get 
- Updated `change_keys` 
    - Updated to use dictionary comprehension to recursively apply `convert` to keys.
    - Removed checks for types `str`, `int`, `float`, `set`, and `tuple` 

**library/test_utils.py**
- Refactored `get_minutes` tests into consolidated test cases within `TestUtilsRefactored`
    - Combined all minute conversion tests into a single method `test_minutes_fixtures`
    - Isolated ISO 8601 duration format tests into `test_iso8601_fixtures`
- Implemented `setUpClass` method in `TestUtilsRefactored` to allow for further testing expansion.
- Updated `test_list_public_methods` to dynamically list and compare public methods of `AbstractScraper`


jayaddison: > Updated `get_host_name` to use .get

This change I'm a little skeptical of - it's an unusual case where I think we'd _want_ the code to break and let us know about it if the `host` info isn't available.  What do you reckon?
jayaddison: Generally looks great, thanks again!

Maybe add 'closes #1004' or 'resolves #1004' to the pull request description to auto-close that bug on merge?
jknndy: > Maybe add 'closes #1004' or 'resolves #1004' to the pull request description to auto-close that bug on merge?

This actually isn't resolved in this update. The FRACTIONS adjustments are only represented in the get_minutes function where the issue in #1004 is related to the ingredients and instructions print outs. Still worth including but I will remove mention of the issue from the PR outline.

Patch:
diff --git a/recipe_scrapers/_utils.py b/recipe_scrapers/_utils.py
index 69458eb5..114bb70c 100644
--- a/recipe_scrapers/_utils.py
+++ b/recipe_scrapers/_utils.py
@@ -9,21 +9,30 @@ import isodate
 from ._exceptions import ElementNotFoundInHtml
 
 FRACTIONS = {
+    "½": 0.5,
+    "⅓": 1 / 3,
+    "⅔": 2 / 3,
     "¼": 0.25,
-    "½": 0.50,
     "¾": 0.75,
-    "⅓": 0.33,
-    "⅔": 0.66,
-    "⅕": 0.20,
-    "⅖": 0.40,
-    "⅗": 0.60,
+    "⅕": 0.2,
+    "⅖": 0.4,
+    "⅗": 0.6,
+    "⅘": 0.8,
+    "⅙": 1 / 6,
+    "⅚": 5 / 6,
+    "⅛": 0.125,
+    "⅜": 0.375,
+    "⅝": 0.625,
+    "⅞": 0.875,
 }
 
 TIME_REGEX = re.compile(
-    r"(\D*(?P<days>\d+)\s*(days|D))?(\D*(?P<hours>[\d.\s/?¼½¾⅓⅔⅕⅖⅗]+)\s*(hours|hrs|hr|h|óra|:))?(\D*(?P<minutes>\d+)\s*(minutes|mins|min|m|perc|$))?",
+    r"(?:\D*(?P<days>\d+)\s*(?:days|D))?"
+    r"(?:\D*(?P<hours>[\d.\s/?¼½¾⅓⅔⅕⅖⅗]+)\s*(?:hours|hrs|hr|h|óra|:))?"
+    r"(?:\D*(?P<minutes>\d+)\s*(?:minutes|mins|min|m|perc|$))?"
+    r"(?:\D*(?P<seconds>\d+)\s*(?:seconds|secs|sec|s))?",
     re.IGNORECASE,
 )
-
 SERVE_REGEX_NUMBER = re.compile(r"(\D*(?P<items>\d+)?\D*)")
 
 SERVE_REGEX_ITEMS = re.compile(
@@ -65,51 +74,70 @@ RECIPE_YIELD_TYPES = (
 
 def _extract_fractional(input_string: str) -> float:
     input_string = input_string.strip()
-    if "/" in input_string:
-        # for example "1 1/2" is matched
-        components = input_string.split(" ")
-        whole_part, fractional_part = components[0], components[-1]
+
+    # Handling mixed numbers with unicode fractions e.g., '1⅔'
+    for unicode_fraction, fraction_part in FRACTIONS.items():
+        if unicode_fraction in input_string:
+            whole_number_part, _, _ = input_string.partition(unicode_fraction)
+
+            whole_number = float(whole_number_part or 0)
+            return whole_number + fraction_part
+
+    if input_string in FRACTIONS:
+        return FRACTIONS[input_string]
+
+    try:
+        return float(input_string)
+    except ValueError:
+        pass
+
+    if " " in input_string and "/" in input_string:
+        whole_part, fractional_part = input_string.split(" ", 1)
         numerator, denominator = fractional_part.split("/")
-        return float(whole_part) + float(int(numerator) / int(denominator))
+        return float(whole_part) + float(numerator) / float(denominator)
 
-    whole_part, fractional_amount = "", 0.0
-    for symbol in input_string:
-        if symbol in FRACTIONS:
-            fractional_amount += FRACTIONS[symbol]
-        else:
-            whole_part += symbol
+    elif "/" in input_string:
+        numerator, denominator = input_string.split("/")
+        return float(numerator) / float(denominator)
 
-    return float(whole_part) + float(fractional_amount)
+    raise ValueError(f"Unrecognized fraction format: '{input_string}'")
 
 
 def get_minutes(element):
     if element is None:
         raise ElementNotFoundInHtml(element)
 
-    # handle integer in string literal
+    # If element is a BeautifulSoup Tag, extract its text content
+    if hasattr(element, "text"):
+        element = element.text
+
     try:
         return int(element)
-    except Exception:
+    except ValueError:
         pass
 
-    if isinstance(element, str):
-        time_text = element
-    else:
-        time_text = element.get_text()
+    if not isinstance(element, str):
+        raise ValueError("Unexpected format for time element")
+    time_text = element
 
     # attempt iso8601 duration parsing
+    if "-" in time_text:  # sometimes formats are like this: '12-15 minutes'
+        _min, _, time_text = time_text.partition("-")
+    if " to " in time_text:  # sometimes formats are like this: '12 to 15 minutes'
+        _min, _to, time_text = time_text.partition(" to ")
+
+    if time_text is None:
+        return None
+
+    # Attempt ISO8601 duration parsing
     if time_text.startswith("P") and "T" in time_text:
         try:
             duration = isodate.parse_duration(time_text)
-            return math.ceil(duration.total_seconds() / 60)
+            total_minutes = math.ceil(duration.total_seconds() / 60)
+            return None if total_minutes == 0 else total_minutes
         except Exception:
             pass
 
-    if "-" in time_text:  # sometimes formats are like this: '12-15 minutes'
-        _min, _, time_text = time_text.partition("-")
-    if " to " in time_text:  # sometimes formats are like this: '12 to 15 minutes'
-        _min, _to, time_text = time_text.partition(" to ")
-
     time_units = TIME_REGEX.search(time_text).groupdict()
     if not any(time_units.values()):
         return None
@@ -117,12 +145,16 @@ def get_minutes(element):
     minutes_matched = time_units.get("minutes")
     hours_matched = time_units.get("hours")
     days_matched = time_units.get("days")
+    seconds_matched = time_units.get("seconds")
 
-    # workaround for formats like: 0D4H45M, that are not a valid iso8601 it seems
     days = float(days_matched) if days_matched else 0
-    hours = float(_extract_fractional(hours_matched)) if hours_matched else 0
+    hours = _extract_fractional(hours_matched) if hours_matched else 0
     minutes = float(minutes_matched) if minutes_matched else 0
-    return minutes + round(hours * 60) + round(days * 24 * 60)
+    seconds = float(seconds_matched) if seconds_matched else 0
+
+    total_minutes = minutes + (hours * 60) + (days * 24 * 60) + (seconds / 60)
+    # Rounding to the nearest whole number, considering seconds
+    return round(total_minutes)
 
 
 def get_yields(element):
@@ -203,7 +235,7 @@ def url_path_to_dict(path):
         r"^"
         r"((?P<schema>.+?)://)?"
         r"((?P<user>.+?)(:(?P<password>.*?))?@)?"
-        r"(?P<host>.*?)"
+        r"(?P<host>[^:/]+)"
         r"(:(?P<port>\d+?))?"
         r"(?P<path>/.*?)?"
         r"(?P<query>[?].*?)?"
@@ -211,9 +243,7 @@ def url_path_to_dict(path):
     )
     regex = re.compile(pattern)
     matches = regex.match(path)
-    url_dict = matches.groupdict() if matches is not None else None
-
-    return url_dict
+    return matches.groupdict() if matches else None
 
 
 def get_host_name(url):
@@ -229,15 +259,13 @@ def change_keys(obj, convert):
     Credit: StackOverflow user 'baldr'
     (https://web.archive.org/web/20201022163147/https://stackoverflow.com/questions/11700705/python-recursively-replace
         -character-in-keys-of-nested-dictionary/33668421)
+
+    Note: with modifications applied.
     """
-    if isinstance(obj, (str, int, float)):
-        return obj
     if isinstance(obj, dict):
-        new = obj.__class__()
-        for k, v in obj.items():
-            new[convert(k)] = change_keys(v, convert)
+        return {convert(k): change_keys(v, convert) for k, v in obj.items()}
     elif isinstance(obj, (list, set, tuple)):
-        new = obj.__class__(change_keys(v, convert) for v in obj)
+        cls = type(obj)
+        return cls(change_keys(item, convert) for item in obj)
     else:
         return obj
-    return new


Test patch:
diff --git a/tests/library/test_utils.py b/tests/library/test_utils.py
index a72822c1..71d1fdce 100644
--- a/tests/library/test_utils.py
+++ b/tests/library/test_utils.py
@@ -1,130 +1,104 @@
 import unittest
 
-from recipe_scrapers._utils import get_minutes
+from recipe_scrapers._utils import _extract_fractional, get_minutes, url_path_to_dict
 
 
 class TestUtils(unittest.TestCase):
-    def test_get_minutes_english_description(self):
-        text = "1 hour 15 mins"
-        self.assertEqual(75, get_minutes(text))
-
-    def test_get_minutes_english_description_with_and(self):
-        text = "1h and 15mins"
-        self.assertEqual(75, get_minutes(text))
-
-    def test_get_minutes_english_abbreviation(self):
-        text = "3h10m"
-        self.assertEqual(190, get_minutes(text))
-
-    def test_get_minutes_short_iso_format(self):
-        text = "PT2H30M"
-        self.assertEqual(150, get_minutes(text))
-
-    def test_get_minutes_long_iso_format(self):
-        text = "P0DT1H10M"
-        self.assertEqual(70, get_minutes(text))
-
-    def test_get_minutes_int_in_string_literal(self):
-        text = "90"
-        self.assertEqual(90, get_minutes(text))
-
-    def test_get_minutes_fraction_in_hours_with_dot_notation(self):
-        text = "1.5 hours"
-        self.assertEqual(90, get_minutes(text))
-
-    def test_get_minutes_integer_days(self):
-        text = "2 days"
-        self.assertEqual(2880, get_minutes(text))
-
-    def test_get_minutes_fraction_with_fraction_unicode_character_halves(self):
-        text = "1½ hours"
-        self.assertEqual(90, get_minutes(text))
-
-    def test_get_minutes_fraction_with_fraction_unicode_character_three_fours(self):
-        text = "1¾ hours"
-        self.assertEqual(105, get_minutes(text))
-
-    def test_get_minutes_fraction_with_fraction_unicode_character_one_fours(self):
-        text = "1¼ hours"
-        self.assertEqual(75, get_minutes(text))
-
-    def test_get_minutes_fraction_with_fraction_unicode_character_two_thirds(self):
-        text = "1⅔ hours"
-        self.assertEqual(100, get_minutes(text))
-
-    def test_get_minutes_fraction_with_fraction_digits_with_slash(self):
-        text = "1 1/2 hours"
-        self.assertEqual(90, get_minutes(text))
-
-    def test_get_minutes_fraction_with_fraction_digits_with_slash_three_fours(self):
-        text = "1 3/4 hours"
-        self.assertEqual(105, get_minutes(text))
-
-    def test_get_minutes_fraction_with_fraction_digits_with_slash_one_fours(self):
-        text = "1 1/4 hours"
-        self.assertEqual(75, get_minutes(text))
-
-    def test_get_minutes_fraction_with_fraction_digits_with_slash_two_thirds(self):
-        text = "1 2/3 hours"
-        self.assertEqual(100, get_minutes(text))
-
-    def test_get_minutes_handles_dashes(self):
-        text = "15 - 20 minutes"
-        self.assertEqual(20, get_minutes(text))
+    @classmethod
+    def setUpClass(cls):
+        cls.iso8601_fixtures = {
+            "PT1H": 60,
+            "PT20M": 20,
+            "PT2H10M": 130,
+            "PT0H9M30S": 10,
+        }
+        cls.minutes_fixtures = [
+            ("1 hour 15 mins", 75),
+            ("1h and 15mins", 75),
+            ("3h10m", 190),
+            ("PT2H30M", 150),
+            ("P0DT1H10M", 70),
+            ("90", 90),
+            ("1.5 hours", 90),
+            ("2 days", 2880),
+            ("1½ hours", 90),
+            ("1¾ hours", 105),
+            ("1¼ hours", 75),
+            ("1⅔ hours", 100),
+            ("1 1/2 hours", 90),
+            ("1 3/4 hours", 105),
+            ("1 1/4 hours", 75),
+            ("1 2/3 hours", 100),
+            ("15 - 20 minutes", 20),
+            ("15 to 20 minutes", 20),
+            ("Pá-Pum", None),
+            ("PT0M", None),
+        ]
 
-    def test_get_minutes_handles_to(self):
-        text = "15 to 20 minutes"
-        self.assertEqual(20, get_minutes(text))
+    def test_minutes_fixtures(self):
+        # Tests for minute related output formats.
+        for text, expected in self.minutes_fixtures:
+            with self.subTest(text=text):
+                self.assertEqual(expected, get_minutes(text))
 
-    def test_get_minutes_imprecise_description(self):
-        text = "Pá-Pum"
-        self.assertEqual(None, get_minutes(text))
+    def test_iso8601_fixtures(self):
+        # Tests for ISO 8601 formatted outputs formats.
+        for text, expected in self.iso8601_fixtures.items():
+            with self.subTest(text=text):
+                self.assertEqual(expected, get_minutes(text))
 
-    iso8601_fixtures = {
-        "PT1H": 60,
-        "PT20M": 20,
-        "PT2H10M": 130,
-        "PT0H9M30S": 10,
-    }
+    def test_split_fractions(self):
+        input_string = "3 1 / 2"
+        expected_result = 3.5
+        self.assertEqual(expected_result, _extract_fractional(input_string))
 
-    def test_get_minutes_handles_iso8601(self):
-        for text, expected_minutes in self.iso8601_fixtures.items():
-            with self.subTest(text=text):
-                self.assertEqual(expected_minutes, get_minutes(text))
+    def test_url_path_to_dict(self):
+        input_path = (
+            "https://recipes:scraper@www.example.com:8080/path/to/resource?key=value"
+        )
+        expected_result = {
+            "schema": "https",
+            "user": "recipes",
+            "password": "scraper",
+            "host": "www.example.com",
+            "port": "8080",
+            "path": "/path/to/resource",
+            "query": "?key=value",
+        }
+        self.assertEqual(expected_result, url_path_to_dict(input_path))
 
     def test_list_public_methods(self):
         from recipe_scrapers import AbstractScraper
 
-        public_method_names = [
+        expected_methods = [
+            "author",
+            "canonical_url",
+            "category",
+            "cook_time",
+            "cuisine",
+            "description",
+            "equipment",
+            "host",
+            "image",
+            "ingredient_groups",
+            "ingredients",
+            "instructions",
+            "instructions_list",
+            "language",
+            "nutrients",
+            "prep_time",
+            "ratings",
+            "reviews",
+            "site_name",
+            "title",
+            "total_time",
+            "yields",
+        ]
+        public_methods = [
             method
             for method in dir(AbstractScraper)
             if callable(getattr(AbstractScraper, method))
-            if not method.startswith("_") and method not in ["soup", "links", "to_json"]
+            and not method.startswith("_")
+            and method not in ["soup", "links", "to_json"]
         ]
-        self.assertEqual(
-            [
-                "author",
-                "canonical_url",
-                "category",
-                "cook_time",
-                "cuisine",
-                "description",
-                "equipment",
-                "host",
-                "image",
-                "ingredient_groups",
-                "ingredients",
-                "instructions",
-                "instructions_list",
-                "language",
-                "nutrients",
-                "prep_time",
-                "ratings",
-                "reviews",
-                "site_name",
-                "title",
-                "total_time",
-                "yields",
-            ],
-            public_method_names,
-        )
+        self.assertEqual((expected_methods), (public_methods))
diff --git a/tests/test_data/kuchynalidla.sk/kuchynalidla.json b/tests/test_data/kuchynalidla.sk/kuchynalidla.json
index 67b85136..492c7d06 100644
--- a/tests/test_data/kuchynalidla.sk/kuchynalidla.json
+++ b/tests/test_data/kuchynalidla.sk/kuchynalidla.json
@@ -86,7 +86,7 @@
   "category": "Hlavné jedlá",
   "title": "Špízy z jahňacieho stehna so zeleninou a s mätovou salsou",
   "total_time": 90,
-  "cook_time": 0,
+  "cook_time": null,
   "prep_time": 90,
   "yields": "6 porcií"
 }
diff --git a/tests/test_data/lecker.de/lecker_1.json b/tests/test_data/lecker.de/lecker_1.json
index 6cd77858..a698ab1b 100644
--- a/tests/test_data/lecker.de/lecker_1.json
+++ b/tests/test_data/lecker.de/lecker_1.json
@@ -55,7 +55,7 @@
     "carbohydrateContent": "7 g",
     "proteinContent": "38 g"
   },
-  "prep_time": 0,
+  "prep_time": null,
   "ratings": 3.94,
   "site_name": null,
   "title": "Gemüsepfanne mit Hähnchen, Zuckerschoten und Brokkoli",
diff --git a/tests/test_data/popsugar.com/popsugar.json b/tests/test_data/popsugar.com/popsugar.json
index e53dc02e..7b7c826a 100644
--- a/tests/test_data/popsugar.com/popsugar.json
+++ b/tests/test_data/popsugar.com/popsugar.json
@@ -63,6 +63,6 @@
   "ratings": 3.5,
   "site_name": "POPSUGAR",
   "title": "Rainbow Pasta",
-  "total_time": 49,
+  "total_time": 50,
   "yields": "6 servings"
 }
diff --git a/tests/test_data/thepioneerwoman.com/thepioneerwoman_2.json b/tests/test_data/thepioneerwoman.com/thepioneerwoman_2.json
index 657facab..9c5855c8 100644
--- a/tests/test_data/thepioneerwoman.com/thepioneerwoman_2.json
+++ b/tests/test_data/thepioneerwoman.com/thepioneerwoman_2.json
@@ -2,7 +2,7 @@
   "author": "Ree Drummond",
   "canonical_url": "https://www.thepioneerwoman.com/food-cooking/recipes/a11885/pumpkin-sheet-cake/",
   "category": "Thanksgiving,baking,comfort food,dessert,main dish,snack",
-  "cook_time": 0,
+  "cook_time": null,
   "cuisine": "American,Comfort Food",
   "description": "Pumpkin sheet cake is moist, heavenly, and the perfect fall recipe. It’s perfect for Thanksgiving—and it’s also perfect for the other 364 days of the year.",
   "host": "thepioneerwoman.com",
diff --git a/tests/test_data/weightwatchers.com/weightwatcherspublic.json b/tests/test_data/weightwatchers.com/weightwatcherspublic.json
index e001e20f..c8c1d4e9 100644
--- a/tests/test_data/weightwatchers.com/weightwatcherspublic.json
+++ b/tests/test_data/weightwatchers.com/weightwatcherspublic.json
@@ -2,7 +2,7 @@
   "author": null,
   "canonical_url": "https://www.weightwatchers.com/de/rezept/kartoffelgulasch/562a9b02873e1afb2a3c4c13",
   "category": "",
-  "cook_time": 0,
+  "cook_time": null,
   "cuisine": "",
   "description": "Das Rezept zaubert ein saftiges, würziges Gericht auf den Tisch und schmeckt garantiert.",
   "host": "weightwatchers.com",