CovidcastRow: address code review #1044

Co-authored-by: Katie Mazaitis <[email protected]>
Co-authored-by: melange396 <[email protected]>

Author: 3 people

integrations/acquisition/covidcast/test_db.py

@@ -31,8 +31,8 @@ def _find_matches_for_row(self, row):
 
     def test_insert_or_update_with_nonempty_load_table(self):
         # make rows
-        a_row = CovidcastRow.make_default_row(time_value=20200202)
-        another_row = CovidcastRow.make_default_row(time_value=20200203, issue=20200203)
+        a_row = CovidcastRow.make_default_row(time_value=2020_02_02)
+        another_row = CovidcastRow.make_default_row(time_value=2020_02_03, issue=2020_02_03)
         # insert one
         self._db.insert_or_update_bulk([a_row])
         # put something into the load table

integrations/client/test_delphi_epidata.py

@@ -68,8 +68,8 @@ def test_covidcast(self):
       )
 
       expected = [
-        row_latest_issue.as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields),
-        rows[-1].as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields)
+        row_latest_issue.as_api_compatibility_row_dict(),
+        rows[-1].as_api_compatibility_row_dict()
       ]
 
       self.assertEqual(response['epidata'], expected)
@@ -88,10 +88,10 @@ def test_covidcast(self):
 
       expected = [{
         rows[0].signal: [
-          row_latest_issue.as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields + ['signal']),
+          row_latest_issue.as_api_compatibility_row_dict(ignore_fields=['signal']),
         ],
         rows[-1].signal: [
-          rows[-1].as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields + ['signal']),
+          rows[-1].as_api_compatibility_row_dict(ignore_fields=['signal']),
         ],
       }]
 
@@ -108,7 +108,7 @@ def test_covidcast(self):
         **self.params_from_row(rows[0])
       )
 
-      expected = [row_latest_issue.as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields)]
+      expected = [row_latest_issue.as_api_compatibility_row_dict()]
 
       # check result
       self.assertEqual(response_1, {
@@ -123,7 +123,7 @@ def test_covidcast(self):
         **self.params_from_row(rows[0], as_of=rows[1].issue)
       )
 
-      expected = [rows[1].as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields)]
+      expected = [rows[1].as_api_compatibility_row_dict()]
 
       # check result
       self.maxDiff=None
@@ -140,8 +140,8 @@ def test_covidcast(self):
       )
 
       expected = [
-        rows[0].as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields),
-        rows[1].as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields)
+        rows[0].as_api_compatibility_row_dict(),
+        rows[1].as_api_compatibility_row_dict()
       ]
 
       # check result
@@ -157,7 +157,7 @@ def test_covidcast(self):
         **self.params_from_row(rows[0], lag=2)
       )
 
-      expected = [row_latest_issue.as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields)]
+      expected = [row_latest_issue.as_api_compatibility_row_dict()]
 
       # check result
       self.assertDictEqual(response_3, {
@@ -231,7 +231,7 @@ def test_geo_value(self):
     self._insert_rows(rows)
 
     counties = [
-      rows[i].as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields) for i in range(N)
+      rows[i].as_api_compatibility_row_dict() for i in range(N)
     ]
 
     def fetch(geo):

integrations/server/test_covidcast.py

@@ -93,7 +93,7 @@ def test_round_trip(self):
     # make the request
     response = self.request_based_on_row(row)
 
-    expected = [row.as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields)]
+    expected = [row.as_api_compatibility_row_dict()]
 
     self.assertEqual(response, {
       'result': 1,
@@ -160,13 +160,13 @@ def test_csv_format(self):
       **{'format':'csv'}
     )
 
-    # TODO: This is a mess because of api.php.
+    # This is a hardcoded mess because of api.php.
     column_order = [
       "geo_value", "signal", "time_value", "direction", "issue", "lag", "missing_value",
       "missing_stderr", "missing_sample_size", "value", "stderr", "sample_size"
     ]
     expected = (
-      row.api_compatibility_row_df
+      row.as_api_compatibility_row_df()
          .assign(direction = None)
          .to_csv(columns=column_order, index=False)
     )
@@ -183,7 +183,7 @@ def test_raw_json_format(self):
     # make the request
     response = self.request_based_on_row(row, **{'format':'json'})
 
-    expected = [row.as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields)]
+    expected = [row.as_api_compatibility_row_dict()]
 
     # assert that the right data came back
     self.assertEqual(response, expected)
@@ -197,7 +197,7 @@ def test_fields(self):
     # limit fields
     response = self.request_based_on_row(row, fields='time_value,geo_value')
 
-    expected = row.as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields)
+    expected = row.as_api_compatibility_row_dict()
     expected_all = {
       'result': 1,
       'epidata': [{
@@ -230,7 +230,7 @@ def test_location_wildcard(self):
 
     # insert placeholder data
     rows = self._insert_placeholder_set_two()
-    expected = [row.as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields) for row in rows[:3]]
+    expected = [row.as_api_compatibility_row_dict() for row in rows[:3]]
     # make the request
     response = self.request_based_on_row(rows[0], geo_value="*")
 
@@ -247,7 +247,7 @@ def test_time_values_wildcard(self):
 
     # insert placeholder data
     rows = self._insert_placeholder_set_three()
-    expected = [row.as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields) for row in rows[:3]]
+    expected = [row.as_api_compatibility_row_dict() for row in rows[:3]]
 
     # make the request
     response = self.request_based_on_row(rows[0], time_values="*")
@@ -265,7 +265,7 @@ def test_issues_wildcard(self):
 
     # insert placeholder data
     rows = self._insert_placeholder_set_five()
-    expected = [row.as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields) for row in rows[:3]]
+    expected = [row.as_api_compatibility_row_dict() for row in rows[:3]]
 
     # make the request
     response = self.request_based_on_row(rows[0], issues="*")
@@ -283,7 +283,7 @@ def test_signal_wildcard(self):
 
     # insert placeholder data
     rows = self._insert_placeholder_set_four()
-    expected_signals = [row.as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields) for row in rows[:3]]
+    expected_signals = [row.as_api_compatibility_row_dict() for row in rows[:3]]
 
     # make the request
     response = self.request_based_on_row(rows[0], signals="*")
@@ -301,7 +301,7 @@ def test_geo_value(self):
 
     # insert placeholder data
     rows = self._insert_placeholder_set_two()
-    expected = [row.as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields) for row in rows[:3]]
+    expected = [row.as_api_compatibility_row_dict() for row in rows[:3]]
 
     def fetch(geo_value):
       # make the request
@@ -337,7 +337,7 @@ def test_location_timeline(self):
 
     # insert placeholder data
     rows = self._insert_placeholder_set_three()
-    expected_timeseries = [row.as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields) for row in rows[:3]]
+    expected_timeseries = [row.as_api_compatibility_row_dict() for row in rows[:3]]
 
     # make the request
     response = self.request_based_on_row(rows[0], time_values='20000101-20000105')
@@ -374,8 +374,7 @@ def test_nullable_columns(self):
 
     # make the request
     response = self.request_based_on_row(row)
-    expected = row.as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields)
-    # expected.update(stderr=None, sample_size=None)
+    expected = row.as_api_compatibility_row_dict()
 
     # assert that the right data came back
     self.assertEqual(response, {
@@ -395,8 +394,8 @@ def test_temporal_partitioning(self):
     self._insert_rows(rows)
 
     # make the request
-    response = self.request_based_on_row(rows[1], time_values="20000101-30010201")
-    expected = [rows[1].as_dict(ignore_fields=CovidcastRow._api_row_compatibility_ignore_fields)]
+    response = self.request_based_on_row(rows[1], time_values="*")
+    expected = [rows[1].as_api_compatibility_row_dict()]
 
     # assert that the right data came back
     self.assertEqual(response, {

integrations/server/test_covidcast_endpoints.py

@@ -39,6 +39,8 @@ def _fetch(self, endpoint="/", is_compatibility=False, **params):
         # make the request
         if is_compatibility:
             url = BASE_URL_OLD
+            # only set endpoint if it's not already set
+            # only set endpoint if it's not already set
             params.setdefault("endpoint", "covidcast")
             if params.get("source"):
                 params.setdefault("data_source", params.get("source"))
@@ -49,7 +51,10 @@ def _fetch(self, endpoint="/", is_compatibility=False, **params):
         return response.json()
 
     def _diff_rows(self, rows: Sequence[float]):
-        return [float(x - y) if x is not None and y is not None else None for x, y in zip(rows[1:], rows[:-1])]
+        return [
+            float(x - y) if x is not None and y is not None else None
+            for x, y in zip(rows[1:], rows[:-1])
+        ]
 
     def _smooth_rows(self, rows: Sequence[float]):
         return [
@@ -59,7 +64,7 @@ def _smooth_rows(self, rows: Sequence[float]):
 
     def test_basic(self):
         """Request a signal from the / endpoint."""
-        rows = [CovidcastRow.make_default_row(time_value=20200401 + i, value=i) for i in range(10)]
+        rows = [CovidcastRow.make_default_row(time_value=2020_04_01 + i, value=i) for i in range(10)]
         first = rows[0]
         self._insert_rows(rows)
 
@@ -68,12 +73,12 @@ def test_basic(self):
             self.assertEqual(out["result"], -1)
 
         with self.subTest("simple"):
-            out = self._fetch("/", signal=first.signal_pair, geo=first.geo_pair, time="day:*")
+            out = self._fetch("/", signal=first.signal_pair(), geo=first.geo_pair(), time="day:*")
             self.assertEqual(len(out["epidata"]), len(rows))
 
     def test_compatibility(self):
         """Request at the /api.php endpoint."""
-        rows = [CovidcastRow.make_default_row(source="src", signal="sig", time_value=20200401 + i, value=i) for i in range(10)]
+        rows = [CovidcastRow.make_default_row(source="src", signal="sig", time_value=2020_04_01 + i, value=i) for i in range(10)]
         first = rows[0]
         self._insert_rows(rows)
 
@@ -82,20 +87,20 @@ def test_compatibility(self):
             self.assertEqual(out["result"], -1)
 
         with self.subTest("simple"):
-            out = self._fetch("/", signal=first.signal_pair, geo=first.geo_pair, time="day:*", is_compatibility=True)
+            out = self._fetch("/", signal=first.signal_pair(), geo=first.geo_pair(), time="day:*", is_compatibility=True)
             self.assertEqual(len(out["epidata"]), len(rows))
 
     def test_trend(self):
         """Request a signal from the /trend endpoint."""
 
         num_rows = 30
-        rows = [CovidcastRow.make_default_row(time_value=20200401 + i, value=i) for i in range(num_rows)]
+        rows = [CovidcastRow.make_default_row(time_value=2020_04_01 + i, value=i) for i in range(num_rows)]
         first = rows[0]
         last = rows[-1]
         ref = rows[num_rows // 2]
         self._insert_rows(rows)
 
-        out = self._fetch("/trend", signal=first.signal_pair, geo=first.geo_pair, date=last.time_value, window="20200401-20201212", basis=ref.time_value)
+        out = self._fetch("/trend", signal=first.signal_pair(), geo=first.geo_pair(), date=last.time_value, window="20200401-20201212", basis=ref.time_value)
 
 
         self.assertEqual(out["result"], 1)
@@ -125,12 +130,12 @@ def test_trendseries(self):
         """Request a signal from the /trendseries endpoint."""
 
         num_rows = 3
-        rows = [CovidcastRow.make_default_row(time_value=20200401 + i, value=num_rows - i) for i in range(num_rows)]
+        rows = [CovidcastRow.make_default_row(time_value=2020_04_01 + i, value=num_rows - i) for i in range(num_rows)]
         first = rows[0]
         last = rows[-1]
         self._insert_rows(rows)
 
-        out = self._fetch("/trendseries", signal=first.signal_pair, geo=first.geo_pair, date=last.time_value, window="20200401-20200410", basis=1)
+        out = self._fetch("/trendseries", signal=first.signal_pair(), geo=first.geo_pair(), date=last.time_value, window="20200401-20200410", basis=1)
 
         self.assertEqual(out["result"], 1)
         self.assertEqual(len(out["epidata"]), 3)
@@ -199,7 +204,7 @@ def test_correlation(self):
         self._insert_rows(other_rows)
         max_lag = 3
 
-        out = self._fetch("/correlation", reference=first.signal_pair, others=other.signal_pair, geo=first.geo_pair, window="20200401-20201212", lag=max_lag)
+        out = self._fetch("/correlation", reference=first.signal_pair(), others=other.signal_pair(), geo=first.geo_pair(), window="20200401-20201212", lag=max_lag)
         self.assertEqual(out["result"], 1)
         df = pd.DataFrame(out["epidata"])
         self.assertEqual(len(df), max_lag * 2 + 1)  # -...0...+
@@ -217,26 +222,27 @@ def test_correlation(self):
     def test_csv(self):
         """Request a signal from the /csv endpoint."""
 
-        rows = [CovidcastRow.make_default_row(time_value=20200401 + i, value=i) for i in range(10)]
+        rows = [CovidcastRow.make_default_row(time_value=2020_04_01 + i, value=i) for i in range(10)]
         first = rows[0]
         self._insert_rows(rows)
 
         response = requests.get(
             f"{BASE_URL}/csv",
-            params=dict(signal=first.signal_pair, start_day="2020-04-01", end_day="2020-12-12", geo_type=first.geo_type),
+            params=dict(signal=first.signal_pair(), start_day="2020-04-01", end_day="2020-12-12", geo_type=first.geo_type),
         )
 
     def test_backfill(self):
         """Request a signal from the /backfill endpoint."""
 
+        TEST_DATE_VALUE = 2020_04_01
         num_rows = 10
-        issue_0 = [CovidcastRow.make_default_row(time_value=20200401 + i, value=i, sample_size=1, lag=0, issue=20200401 + i) for i in range(num_rows)]
-        issue_1 = [CovidcastRow.make_default_row(time_value=20200401 + i, value=i + 1, sample_size=2, lag=1, issue=20200401 + i + 1) for i in range(num_rows)]
-        last_issue = [CovidcastRow.make_default_row(time_value=20200401 + i, value=i + 2, sample_size=3, lag=2, issue=20200401 + i + 2) for i in range(num_rows)] # <-- the latest issues
+        issue_0 = [CovidcastRow.make_default_row(time_value=TEST_DATE_VALUE + i, value=i, sample_size=1, lag=0, issue=TEST_DATE_VALUE + i) for i in range(num_rows)]
+        issue_1 = [CovidcastRow.make_default_row(time_value=TEST_DATE_VALUE + i, value=i + 1, sample_size=2, lag=1, issue=TEST_DATE_VALUE + i + 1) for i in range(num_rows)]
+        last_issue = [CovidcastRow.make_default_row(time_value=TEST_DATE_VALUE + i, value=i + 2, sample_size=3, lag=2, issue=TEST_DATE_VALUE + i + 2) for i in range(num_rows)] # <-- the latest issues
         self._insert_rows([*issue_0, *issue_1, *last_issue])
         first = issue_0[0]
 
-        out = self._fetch("/backfill", signal=first.signal_pair, geo=first.geo_pair, time="day:20200401-20201212", anchor_lag=3)
+        out = self._fetch("/backfill", signal=first.signal_pair(), geo=first.geo_pair(), time="day:20200401-20201212", anchor_lag=3)
         self.assertEqual(out["result"], 1)
         df = pd.DataFrame(out["epidata"])
         self.assertEqual(len(df), 3 * num_rows)  # num issues
@@ -258,7 +264,7 @@ def test_meta(self):
         """Request a signal from the /meta endpoint."""
 
         num_rows = 10
-        rows = [CovidcastRow.make_default_row(time_value=20200401 + i, value=i, source="fb-survey", signal="smoothed_cli") for i in range(num_rows)]
+        rows = [CovidcastRow.make_default_row(time_value=2020_04_01 + i, value=i, source="fb-survey", signal="smoothed_cli") for i in range(num_rows)]
         self._insert_rows(rows)
         first = rows[0]
         last = rows[-1]
@@ -298,23 +304,23 @@ def test_coverage(self):
         """Request a signal from the /coverage endpoint."""
 
         num_geos_per_date = [10, 20, 30, 40, 44]
-        dates = [20200401 + i for i in range(len(num_geos_per_date))]
+        dates = [2020_04_01 + i for i in range(len(num_geos_per_date))]
         rows = [CovidcastRow.make_default_row(time_value=dates[i], value=i, geo_value=str(geo_value)) for i, num_geo in enumerate(num_geos_per_date) for geo_value in range(num_geo)]
         self._insert_rows(rows)
         first = rows[0]
 
         with self.subTest("default"):
-            out = self._fetch("/coverage", signal=first.signal_pair, geo_type=first.geo_type, latest=dates[-1], format="json")
+            out = self._fetch("/coverage", signal=first.signal_pair(), geo_type=first.geo_type, latest=dates[-1], format="json")
             self.assertEqual(len(out), len(num_geos_per_date))
             self.assertEqual([o["time_value"] for o in out], dates)
             self.assertEqual([o["count"] for o in out], num_geos_per_date)
 
         with self.subTest("specify window"):
-            out = self._fetch("/coverage", signal=first.signal_pair, geo_type=first.geo_type, window=f"{dates[0]}-{dates[1]}", format="json")
+            out = self._fetch("/coverage", signal=first.signal_pair(), geo_type=first.geo_type, window=f"{dates[0]}-{dates[1]}", format="json")
             self.assertEqual(len(out), 2)
             self.assertEqual([o["time_value"] for o in out], dates[:2])
             self.assertEqual([o["count"] for o in out], num_geos_per_date[:2])
 
         with self.subTest("invalid geo_type"):
-            out = self._fetch("/coverage", signal=first.signal_pair, geo_type="doesnt_exist", format="json")
+            out = self._fetch("/coverage", signal=first.signal_pair(), geo_type="doesnt_exist", format="json")
             self.assertEqual(len(out), 0)