Merge branch 'main' into change-smooth-refactor

Author: chinandrew

.github/CONTRIBUTING.md

@@ -61,7 +61,7 @@ git checkout -b dev-my-feature-branch
 
 ### Creating your indicator
 
-Create a directory for your new indicator by making a copy of `_template_r` or `_template_python` depending on the programming language you intend to use. The template copies of `README.md` and `REVIEW.md` include the minimum requirements for code structure, documentation, linting, testing, and method of configuration. Beyond that, we don't have any established restrictions on implementation; you can look at other existing indicators see some examples of code layout, organization, and general approach.
+Create a directory for your new indicator by making a copy of `_template_r` or `_template_python` depending on the programming language you intend to use. If using Python, add the name of the directory to the list found in `jobs > build > strategy > matrix > packages` in `.github/workflows/python-ci.yml`, which will enable automated checks for your indicator when you make PRs. The template copies of `README.md` and `REVIEW.md` include the minimum requirements for code structure, documentation, linting, testing, and method of configuration. Beyond that, we don't have any established restrictions on implementation; you can look at other existing indicators see some examples of code layout, organization, and general approach.
 
 - Consult your peers with questions! :handshake:
 

.github/pull_request_template.md

@@ -0,0 +1,9 @@
+### Description
+Type of change (bug fix, new feature, etc), brief description, and motivation for these changes.
+
+### Changelog
+- change1
+- change2
+
+### Fixes 
+- Fixes #(issue)

.github/workflows/python-ci.yml

@@ -7,7 +7,7 @@ on:
   push:
     branches: [ main ]
   pull_request:
-    types: [ opened, synchronized, reopened, ready_for_review ]
+    types: [ opened, synchronize, reopened, ready_for_review ]
     branches: [ main ]
 
 jobs:

_delphi_utils_python/delphi_utils/archive.py

@@ -104,7 +104,8 @@ def run_module(archive_type: str,
     Parameters
     ----------
     archive_type: str
-        Type of ArchiveDiffer to run.  Must be one of ["git", "s3"] which correspond to `GitArchiveDiffer` and `S3ArchiveDiffer`, respectively.
+        Type of ArchiveDiffer to run.  Must be one of ["git", "s3"] which correspond to
+        `GitArchiveDiffer` and `S3ArchiveDiffer`, respectively.
     cache_dir: str
         The directory for storing most recent archived/uploaded CSVs to start diffing from.
     export_dir: str
@@ -351,7 +352,7 @@ def update_cache(self):
 
         self._cache_updated = True
 
-    def archive_exports(self,
+    def archive_exports(self,  # pylint: disable=arguments-differ
         exported_files: Files,
         update_cache: bool = True,
         update_s3: bool = True

_delphi_utils_python/delphi_utils/export.py

@@ -1,3 +1,4 @@
+"""Export data in the format expected by the Delphi API."""
 # -*- coding: utf-8 -*-
 from datetime import datetime
 from os.path import join

_delphi_utils_python/delphi_utils/geomap.py

@@ -9,7 +9,7 @@
 - remove deprecated functions once integration into JHU and Quidel is refactored
   see: https://github.com/cmu-delphi/covidcast-indicators/issues/283
 """
-
+# pylint: disable=too-many-lines
 from os.path import join
 import warnings
 import pkg_resources
@@ -41,7 +41,7 @@
 }
 
 
-class GeoMapper:
+class GeoMapper:  # pylint: disable=too-many-public-methods
     """Geo mapping tools commonly used in Delphi.
 
     The GeoMapper class provides utility functions for translating between different
@@ -301,7 +301,7 @@ def add_geocode(
                 df[from_col] = df[from_col].astype(str)
 
         # Assuming that the passed-in records are all United States data, at the moment
-        if (from_code, new_code) in [("fips", "nation"), ("zip", "nation")]:
+        if (from_code, new_code) in [("fips", "nation"), ("zip", "nation")]: # pylint: disable=no-else-return
             df[new_col] = df[from_col].apply(lambda x: "us")
             return df
         elif new_code == "nation":
@@ -724,7 +724,7 @@ def convert_state_code_to_state_id(
         )
 
         state_table = self._load_crosswalk(from_code="state", to_code="state")
-        state_table = state_table[["state_code", "state_id"]].rename(
+        state_table = state_table[["state_code", "state_id"]].rename(  # pylint: disable=unsubscriptable-object
             columns={"state_id": state_id_col}
         )
         data = data.copy()
@@ -796,7 +796,7 @@ def convert_zip_to_hrr(self, data, zip_col="zip", hrr_col="hrr"):
         return data
 
     def convert_zip_to_msa(
-        self, data, zip_col="zip", msa_col="msa", date_col="date", count_cols=None
+        self, data, zip_col="zip", date_col="date", count_cols=None
     ):
         """DEPRECATED."""
         warnings.warn(
@@ -828,7 +828,6 @@ def zip_to_msa(
         data = self.convert_zip_to_msa(
             data,
             zip_col=zip_col,
-            msa_col=msa_col,
             date_col=date_col,
             count_cols=count_cols,
         )

_delphi_utils_python/delphi_utils/smooth.py

@@ -18,7 +18,7 @@
 import pandas as pd
 
 
-class Smoother:
+class Smoother:  # pylint: disable=too-many-instance-attributes
     """Smoother class.
 
     This is the smoothing utility class. This class holds the parameter settings for its smoother
@@ -78,8 +78,8 @@ class Smoother:
     Methods
     ----------
     smooth: np.ndarray or pd.Series
-        Takes a 1D signal and returns a smoothed version. The input and the output have the same
-        length and type.
+        Takes a 1D signal and returns a smoothed version.
+        The input and the output have the same length and type.
 
     Example Usage
     -------------
@@ -260,20 +260,21 @@ def left_gauss_linear_smoother(self, signal):
         )
         n = len(signal)
         signal_smoothed = np.zeros_like(signal)
-        A = np.vstack([np.ones(n), np.arange(n)]).T  # the regression design matrix
+        # A is the regression design matrix
+        A = np.vstack([np.ones(n), np.arange(n)]).T  # pylint: disable=invalid-name
         for idx in range(n):
             weights = np.exp(
                 -((np.arange(idx + 1) - idx) ** 2) / self.gaussian_bandwidth
             )
-            AwA = np.dot(A[: (idx + 1), :].T * weights, A[: (idx + 1), :])
-            Awy = np.dot(
+            AwA = np.dot(A[: (idx + 1), :].T * weights, A[: (idx + 1), :])  # pylint: disable=invalid-name
+            Awy = np.dot(  # pylint: disable=invalid-name
                 A[: (idx + 1), :].T * weights, signal[: (idx + 1)].reshape(-1, 1)
             )
             try:
                 beta = np.linalg.solve(AwA, Awy)
                 signal_smoothed[idx] = np.dot(A[: (idx + 1), :], beta)[-1]
             except np.linalg.LinAlgError:
-                signal_smoothed[idx] = signal[idx] if self.impute else np.nan
+                signal_smoothed[idx] = signal[idx] if self.impute else np.nan  # pylint: disable=using-constant-test
         if self.minval is not None:
             signal_smoothed[signal_smoothed <= self.minval] = self.minval
         return signal_smoothed
@@ -336,7 +337,7 @@ def savgol_coeffs(self, nl, nr):
         if nr > 0:
             raise warnings.warn("The filter is no longer causal.")
 
-        A = np.vstack(
+        A = np.vstack(  # pylint: disable=invalid-name
             [np.arange(nl, nr + 1) ** j for j in range(self.poly_fit_degree + 1)]
         ).T
 
@@ -380,7 +381,7 @@ def savgol_smoother(self, signal):
         # - shortened_window (default) applies savgol with a smaller window to do the fit
         # - identity keeps the original signal (doesn't smooth)
         # - nan writes nans
-        if self.boundary_method == "shortened_window":
+        if self.boundary_method == "shortened_window":  # pylint: disable=no-else-return
             for ix in range(len(self.coeffs)):
                 if ix == 0:
                     signal_smoothed[ix] = signal[ix]

_delphi_utils_python/delphi_utils/utils.py

@@ -1,3 +1,4 @@
+"""Read parameter files containing configuration information."""
 # -*- coding: utf-8 -*-
 from json import load
 from os.path import exists

_delphi_utils_python/tests/test_signal.py

@@ -1,7 +1,8 @@
 """Tests for delphi_utils.signal."""
 from unittest.mock import patch
-import pandas as pd
 
+import pandas as pd
+import pytest
 from delphi_utils.signal import add_prefix, public_signal
 
 # Constants for mocking out the call to `covidcast.metadata` within `public_signal()`.
@@ -14,12 +15,17 @@ class TestSignal:
     def test_add_prefix_to_all(self):
         """Tests that `add_prefix()` derives work-in-progress names for all input signals."""
         assert add_prefix(["sig1", "sig3"], True, prefix="wip_") == ["wip_sig1", "wip_sig3"]
-    
+
     def test_add_prefix_to_specified(self):
         """Tests that `add_prefix()` derives work-in-progress names for specified signals."""
         assert add_prefix(["sig1", "sig2", "sig3"], ["sig2"], prefix="wip_") ==\
             ["sig1", "wip_sig2", "sig3"]
-    
+
+    def test_invalid_prefix_input(self):
+        """Tests that `add_prefix()` raises a ValueError when invalid input is given."""
+        with pytest.raises(ValueError):
+            add_prefix(None, None)
+
     @patch("covidcast.metadata")
     def test_add_prefix_to_non_public(self, metadata):
         """Tests that `add_prefix()` derives work-in-progress names for non-public signals."""

_template_python/README.md

@@ -7,15 +7,15 @@
 The indicator is run by directly executing the Python module contained in this
 directory. The safest way to do this is to create a virtual environment,
 installed the common DELPHI tools, and then install the module and its
-dependencies. To do this, run the following code from this directory:
+dependencies. To do this, run the following command from this directory:
 
 ```
-python -m venv env
-source env/bin/activate
-pip install ../_delphi_utils_python/.
-pip install .
+make install
 ```
 
+This command will install the package in editable mode, so you can make changes that
+will automatically propagate to the installed package. 
+
 All of the user-changable parameters are stored in `params.json`. To execute
 the module and produce the output datasets (by default, in `receiving`), run
 the following:
@@ -24,34 +24,39 @@ the following:
 env/bin/python -m delphi_NAME
 ```
 
-Once you are finished with the code, you can deactivate the virtual environment
-and (optionally) remove the environment itself.
+If you want to enter the virtual environment in your shell, 
+you can run `source env/bin/activate`. Run `deactivate` to leave the virtual environment. 
+
+Once you are finished, you can remove the virtual environment and 
+params file with the following:
 
 ```
-deactivate
-rm -r env
+make clean
 ```
 
 ## Testing the code
 
-To do a static test of the code style, it is recommended to run **pylint** on
-the module. To do this, run the following from the main module directory:
+To run static tests of the code style, run the following command:
 
 ```
-env/bin/pylint delphi_NAME
+make lint
 ```
 
-The most aggressive checks are turned off; only relatively important issues
-should be raised and they should be manually checked (or better, fixed).
-
 Unit tests are also included in the module. To execute these, run the following
 command from this directory:
 
 ```
-(cd tests && ../env/bin/pytest --cov=delphi_NAME --cov-report=term-missing)
+make test
+```
+
+To run individual tests, run the following:
+
+```
+(cd tests && ../env/bin/pytest <your_test>.py --cov=delphi_NAME --cov-report=term-missing)
 ```
 
 The output will show the number of unit tests that passed and failed, along
-with the percentage of code covered by the tests. None of the tests should
-fail and the code lines that are not covered by unit tests should be small and
-should not include critical sub-routines.
+with the percentage of code covered by the tests. 
+
+None of the linting or unit tests should fail, and the code lines that are not covered by unit tests should be small and
+should not include critical sub-routines. 

cdc_covidnet/README.md

@@ -10,12 +10,12 @@ installed the common DELPHI tools, and then install the module and its
 dependencies. To do this, run the following code from this directory:
 
 ```
-python -m venv env
-source env/bin/activate
-pip install ../_delphi_utils_python/.
-pip install .
+make install
 ```
 
+This command will install the package in editable mode, so you can make changes that
+will automatically propagate to the installed package. 
+
 All of the user-changable parameters are stored in `params.json`.
 
 The module has to request for each participating state's hospitalization data
@@ -32,35 +32,39 @@ the following:
 env/bin/python -m delphi_cdc_covidnet
 ```
 
-Once you are finished with the code, you can deactivate the virtual environment
-and (optionally) remove the environment itself.
+If you want to enter the virtual environment in your shell, 
+you can run `source env/bin/activate`. Run `deactivate` to leave the virtual environment. 
+
+Once you are finished, you can remove the virtual environment and 
+params file with the following:
 
 ```
-deactivate
-rm -r env
+make clean
 ```
 
 ## Testing the code
 
-To do a static test of the code style, it is recommended to run **pylint** on
-the module. To do this, run the following from the main module directory:
+To run static tests of the code style, run the following command:
 
 ```
-env/bin/pylint delphi_cdc_covidnet
+make lint
 ```
 
-The most aggressive checks are turned off; only relatively important issues
-should be raised and they should be manually checked (or better, fixed).
-
 Unit tests are also included in the module. To execute these, run the following
 command from this directory:
-(Note: the following command requires python 3.8, having any version less than 3.8 might
-fail some test cases. Please install it before running.)
+
 ```
-(cd tests && ../env/bin/pytest --cov=delphi_cdc_covidnet --cov-report=term-missing)
+make test
+```
+
+To run individual tests, run the following:
+
+```
+(cd tests && ../env/bin/pytest <your_test>.py --cov=delphi_cdc_covidnet --cov-report=term-missing)
 ```
 
 The output will show the number of unit tests that passed and failed, along
-with the percentage of code covered by the tests. None of the tests should
-fail and the code lines that are not covered by unit tests should be small and
-should not include critical sub-routines.
+with the percentage of code covered by the tests. 
+
+None of the linting or unit tests should fail, and the code lines that are not covered by unit tests should be small and
+should not include critical sub-routines.