Merge pull request #31 from OCHA-DAP/yahoo_api_fix

Better fix for Yahoo API error

src/hdx/location/currency.py

@@ -172,6 +172,39 @@ def _get_primary_rates_data(
         except (DownloadError, KeyError):
             return None
 
+    @classmethod
+    def _get_adjclose(cls, indicators: Dict) -> Optional[float]:
+        """
+        Get the adjusted close fx rate from the indicators dictionary returned
+        from the Yahoo API.
+
+        Args:
+            indicators (Dict): Indicators dictionary from Yahoo API
+
+        Returns:
+            Optional[float]: Adjusted close fx rate or None
+        """
+        adjclose = indicators["adjclose"][0].get("adjclose")
+        if adjclose is None:
+            return None
+        adjclose = adjclose[0]
+        # compare with high and low to reveal errors from Yahoo feed
+        quote = indicators["quote"][0]
+        high = quote.get("high")
+        low = quote.get("low")
+        if high and low:
+            high = high[0]
+            low = low[0]
+            if adjclose > high:
+                diff = adjclose / high
+                if diff > 1.1:
+                    adjclose = low + (high - low) / 2
+            elif adjclose < low:
+                diff = low / adjclose
+                if diff > 1.1:
+                    adjclose = low + (high - low) / 2
+        return adjclose
+
     @classmethod
     def _get_primary_rate(
         cls, currency: str, timestamp: Optional[int] = None
@@ -201,21 +234,7 @@ def _get_primary_rate(
         if not data:
             return None
         if get_close:
-            indicators = data["indicators"]
-            adjclose = indicators["adjclose"][0].get("adjclose")
-            if adjclose is None:
-                return None
-            adjclose = adjclose[0]
-            # compare with high and low to reveal errors from Yahoo feed
-            quote = indicators["quote"][0]
-            high = quote.get("high")
-            low = quote.get("low")
-            if high and low:
-                high = high[0]
-                low = low[0]
-                if adjclose > high or adjclose < low:
-                    adjclose = low + (high - low) / 2
-            return adjclose
+            return cls._get_adjclose(data["indicators"])
         return data["meta"]["regularMarketPrice"]
 
     @classmethod

tests/hdx/location/test_currency.py

@@ -248,7 +248,7 @@ def test_get_historic_value_in_usd(
         # 0.761817697025102 + (0.776276975624903 - 0.761817697025102) * (1582156800-1580428800) / (1582934400 - 1580428800)
         assert Currency.get_historic_rate("gbp", date) == 0.7717896133008268
 
-    def test_broken_rate(self, retrievers, secondary_historic_url):
+    def test_broken_rates(self, retrievers, secondary_historic_url):
         Currency._no_historic = False
         Currency.setup(secondary_historic_url=secondary_historic_url)
         # Without the checking against high and low returned by Yahoo API, this
@@ -257,3 +257,75 @@ def test_broken_rate(self, retrievers, secondary_historic_url):
             Currency.get_historic_rate("NGN", parse_date("2017-02-15"))
             == 314.5
         )
+        # Without the checking against high and low returned by Yahoo API, this
+        # returned 0.10000000149011612
+        assert (
+            Currency.get_historic_rate("YER", parse_date("2016-09-15"))
+            == 249.7249984741211
+        )
+        # Since the adjclose is not too different from the low and high,
+        # despite being outside their range, we use adjclose
+        assert (
+            Currency.get_historic_rate("XAF", parse_date("2022-04-14"))
+            == 605.5509643554688
+        )
+        # Since the adjclose is not too different from the low and high,
+        # despite being outside their range, we use adjclose
+        assert (
+            Currency.get_historic_rate("XAF", parse_date("2022-04-15"))
+            == 601.632568359375
+        )
+
+    def test_get_adjclose(self):
+        indicators = {
+            "adjclose": [{"adjclose": [3.140000104904175]}],
+            "quote": [
+                {
+                    "close": [3.140000104904175],
+                    "high": [315.0],
+                    "low": [314.0],
+                    "open": [315.0],
+                    "volume": [0],
+                }
+            ],
+        }
+        assert Currency._get_adjclose(indicators) == 314.5
+        indicators = {
+            "adjclose": [{"adjclose": [605.5509643554688]}],
+            "quote": [
+                {
+                    "close": [605.5509643554688],
+                    "high": [602.6080932617188],
+                    "low": [601.632568359375],
+                    "open": [602.6080932617188],
+                    "volume": [0],
+                }
+            ],
+        }
+        assert Currency._get_adjclose(indicators) == 605.5509643554688
+        indicators = {
+            "adjclose": [{"adjclose": [601.632568359375]}],
+            "quote": [
+                {
+                    "close": [601.632568359375],
+                    "high": [606.8197631835938],
+                    "low": [606.8197631835938],
+                    "open": [606.8197631835938],
+                    "volume": [0],
+                }
+            ],
+        }
+        assert Currency._get_adjclose(indicators) == 601.632568359375
+        indicators = {
+            "adjclose": [{"adjclose": [314.0000104904175]}],
+            "quote": [
+                {
+                    "close": [314.0000104904175],
+                    "high": [3.150],
+                    "low": [3.140],
+                    "open": [3.150],
+                    "volume": [0],
+                }
+            ],
+        }
+        assert Currency._get_adjclose(indicators) == 3.145