Merge pull request #494 from CodeForPhilly/PhilMiller/491-cli-sync

jlubken · web-flow · commit fe8f41a488b3 · 2020-04-06T10:16:08.000-04:00
Adapt Parameters and CLI to mostly automatically stay in sync
diff --git a/src/penn_chime/cli.py b/src/penn_chime/cli.py
@@ -4,12 +4,11 @@
     Action,
     ArgumentParser,
 )
-from datetime import datetime
 
 from pandas import DataFrame
 
 from .constants import CHANGE_DATE
-from .parameters import Parameters, Disposition
+from .parameters import Parameters, Disposition, ACCEPTED_PARAMETERS
 from .models import SimSirModel as Model
 
 
@@ -21,8 +20,16 @@ def __call__(self, parser, namespace, values, option_string=None):
             parser.parse_args(f.read().split(), namespace)
 
 
-def cast_date(string):
-    return datetime.strptime(string, '%Y-%m-%d').date()
+def declarative_validator(cast):
+    """Validator."""
+
+    def validate(string):
+        """Validate."""
+        if string == '' and cast != str:
+            return None
+        return cast(string)
+
+    return validate
 
 
 def validator(arg, cast, min_value, max_value, required=True):
@@ -49,31 +56,18 @@ def parse_args():
     parser = ArgumentParser(description=f"penn_chime: {CHANGE_DATE}")
     parser.add_argument("--file", type=open, action=FromFile)
 
+    for name, (params_validator, default, cast, help) in ACCEPTED_PARAMETERS.items():
+        if cast is None:
+            continue
+
+        parser.add_argument(
+            "--" + name.replace('_', '-'),
+            type=declarative_validator(cast),
+            default=default,
+            help=help
+        )
+
     for arg, cast, min_value, max_value, help, required in (
-        (
-            "--current-hospitalized",
-            int,
-            0,
-            None,
-            "Currently hospitalized COVID-19 patients (>= 0)",
-            True,
-        ),
-        (
-            "--date-first-hospitalized",
-            cast_date,
-            None,
-            None,
-            "Current date",
-            False,
-        ),
-        (
-            "--doubling-time",
-            float,
-            0.0,
-            None,
-            "Doubling time before social distancing (days)",
-            True,
-        ),
         ("--hospitalized-days", int, 0, None, "Average hospital length of stay (in days)", True),
         (
             "--hospitalized-rate",
@@ -85,25 +79,7 @@ def parse_args():
         ),
         ("--icu-days", int, 0, None, "Average days in ICU", True),
         ("--icu-rate", float, 0.0, 1.0, "ICU rate: 0.0 - 1.0", True),
-        (
-            "--market_share",
-            float,
-            0.00001,
-            1.0,
-            "Hospital market share (0.00001 - 1.0)",
-            True,
-        ),
-        ("--infectious-days", float, 0.0, None, "Infectious days", True),
-        ("--n-days", int, 0, None, "Number of days to project >= 0", True),
-        (
-            "--relative-contact-rate",
-            float,
-            0.0,
-            1.0,
-            "Social distancing reduction rate: 0.0 - 1.0",
-            True,
-        ),
-        ("--population", int, 1, None, "Regional population >= 1", True),
+
         ("--ventilated-days", int, 0, None, "Average days on ventilator", True),
         ("--ventilated-rate", float, 0.0, 1.0, "Ventilated Rate: 0.0 - 1.0", True),
     ):
@@ -119,19 +95,24 @@ def main():
     """Main."""
     a = parse_args()
 
+    del a.file
+
+    hospitalized = Disposition(a.hospitalized_rate, a.hospitalized_days)
+    icu = Disposition(a.icu_rate, a.icu_days)
+    ventilated = Disposition(a.ventilated_rate, a.ventilated_days)
+
+    del a.hospitalized_days
+    del a.hospitalized_rate
+    del a.icu_days
+    del a.icu_rate
+    del a.ventilated_days
+    del a.ventilated_rate
+
     p = Parameters(
-        current_hospitalized=a.current_hospitalized,
-        date_first_hospitalized=a.date_first_hospitalized,
-        doubling_time=a.doubling_time,
-        infectious_days=a.infectious_days,
-        market_share=a.market_share,
-        n_days=a.n_days,
-        relative_contact_rate=a.relative_contact_rate,
-        population=a.population,
-
-        hospitalized=Disposition(a.hospitalized_rate, a.hospitalized_days),
-        icu=Disposition(a.icu_rate, a.icu_days),
-        ventilated=Disposition(a.ventilated_rate, a.ventilated_days),
+        hospitalized=hospitalized,
+        icu=icu,
+        ventilated=ventilated,
+        **vars(a)
     )
 
     m = Model(p)
diff --git a/src/penn_chime/parameters.py b/src/penn_chime/parameters.py
@@ -5,11 +5,11 @@
 """
 
 from collections import namedtuple
-from datetime import date
+from datetime import date, datetime
 from typing import Optional
 
 from .validators import (
-    Positive, OptionalStrictlyPositive, StrictlyPositive, Rate, Date, OptionalDate
+    OptionalValue, Positive, OptionalStrictlyPositive, StrictlyPositive, Rate, Date, OptionalDate, ValDisposition
     )
 
 # Parameters for each disposition (hospitalized, icu, ventilated)
@@ -45,62 +45,60 @@ def __init__(self, **kwargs):
         self.population = population
 
 
-class Parameters:
-    """Parameters."""
+def cast_date(string):
+    return datetime.strptime(string, '%Y-%m-%d').date()
+
+
+# Dictionary from parameter names to Tuples containing (validator, default value, cast function, help text)
+ACCEPTED_PARAMETERS = {
+    "current_hospitalized": (Positive, None, int, "Currently hospitalized COVID-19 patients (>= 0)"),
+    "current_date": (OptionalDate, None, cast_date, "Date on which the forecast should be based"),
+    "date_first_hospitalized": (OptionalDate, None, cast_date, "Date the first patient was hospitalized"),
+    "doubling_time": (OptionalStrictlyPositive, None, float, "Doubling time before social distancing (days)"),
+    "relative_contact_rate": (Rate, None, float, "Social distancing reduction rate: 0.0 - 1.0"),
+    "mitigation_date": (OptionalDate, None, cast_date, "Date on which social distancing measures too effect"),
+    "infectious_days": (StrictlyPositive, 14, int, "Infectious days"),
+    "market_share": (Rate, 1.0, float, "Hospital market share (0.00001 - 1.0)"),
+    "max_y_axis": (OptionalStrictlyPositive, None, int, None),
+    "n_days": (StrictlyPositive, 100, int, "Number of days to project >= 0"),
+    "recovered": (Positive, 0, int, "Number of patients already recovered (not yet implemented)"),
+    "population": (OptionalStrictlyPositive, None, int, "Regional population >= 1"),
+    "region": (OptionalValue, None, None, "No help available"),
+
+    "hospitalized": (ValDisposition, None, None, None),
+    "icu": (ValDisposition, None, None, None),
+    "ventilated": (ValDisposition, None, None, None),
+}
 
-    def __init__(
-        self,
-        *,
-        current_hospitalized: int,
-        hospitalized: Disposition,
-        icu: Disposition,
-        relative_contact_rate: float,
-        mitigation_date: Optional[date] = None,
-        ventilated: Disposition,
-        current_date: Optional[date] = None,
-        date_first_hospitalized: Optional[date] = None,
-        doubling_time: Optional[float] = None,
-        infectious_days: int = 14,
-        market_share: float = 1.0,
-        max_y_axis: Optional[int] = None,
-        n_days: int = 100,
-        population: Optional[int] = None,
-        recovered: int = 0,
-        region: Optional[Regions] = None,
-    ):
-        self.current_hospitalized = Positive(value=current_hospitalized)
-        Rate(value=hospitalized.rate), Rate(value=icu.rate), Rate(value=ventilated.rate)
-        StrictlyPositive(value=hospitalized.days), StrictlyPositive(value=icu.days),
-        StrictlyPositive(value=ventilated.days)
-
-        self.hospitalized = hospitalized
-        self.icu = icu
-        self.ventilated = ventilated
-
-        if region is not None and population is None:
-            self.region = region
-            self.population = StrictlyPositive(value=region.population)
-        elif population is not None:
-            self.region = None
-            self.population = StrictlyPositive(value=population)
-        else:
-            raise AssertionError('population or regions must be provided.')
 
-        if current_date is None:
-            current_date = date.today()
-        self.current_date = Date(value=current_date)
+class Parameters:
+    """Parameters."""
 
-        self.date_first_hospitalized = OptionalDate(value=date_first_hospitalized)
-        self.doubling_time = OptionalStrictlyPositive(value=doubling_time)
+    def __init__(self, **kwargs):
+        passed_and_default_parameters = {}
+        for key, value in kwargs.items():
+            if key not in ACCEPTED_PARAMETERS:
+                raise ValueError(f"Unexpected parameter {key}")
+            passed_and_default_parameters[key] = value
+
+        for key, (validator, default_value, cast, help) in ACCEPTED_PARAMETERS.items():
+            if key not in passed_and_default_parameters:
+                passed_and_default_parameters[key] = default_value
+
+        for key, value in passed_and_default_parameters.items():
+            validator = ACCEPTED_PARAMETERS[key][0]
+            try:
+                validator(value=value)
+            except (TypeError, ValueError) as ve:
+                raise ValueError(f"For parameter '{key}', with value '{value}', validation returned error \"{ve}\"")
+            setattr(self, key, value)
 
-        self.relative_contact_rate = Rate(value=relative_contact_rate)
-        self.mitigation_date = OptionalDate(value=mitigation_date)
+        if self.region is  None and self.population is None:
+            raise AssertionError('population or regions must be provided.')
 
-        self.infectious_days = StrictlyPositive(value=infectious_days)
-        self.market_share = Rate(value=market_share)
-        self.max_y_axis = OptionalStrictlyPositive(value=max_y_axis)
-        self.n_days = StrictlyPositive(value=n_days)
-        self.recovered = Positive(value=recovered)
+        if self.current_date is None:
+            self.current_date = date.today()
+        Date(value=self.current_date)
 
         self.labels = {
             "hospitalized": "Hospitalized",
@@ -114,7 +112,7 @@ def __init__(
         }
 
         self.dispositions = {
-            "hospitalized": hospitalized,
-            "icu": icu,
-            "ventilated": ventilated,
+            "hospitalized": self.hospitalized,
+            "icu": self.icu,
+            "ventilated": self.ventilated,
         }
diff --git a/src/penn_chime/validators/__init__.py b/src/penn_chime/validators/__init__.py
@@ -1,14 +1,14 @@
 """the callable validator design pattern"""
 
-from .validators import Bounded, OptionalBounded, Rate, Date, OptionalDate
-
-EPSILON = 1.e-7
+from .validators import EPSILON, OptionalValue, Bounded, OptionalBounded, Rate, Date, OptionalDate, ValDisposition
 
+OptionalValue = OptionalValue()
 OptionalStrictlyPositive = OptionalBounded(lower_bound=EPSILON)
 StrictlyPositive = Bounded(lower_bound=EPSILON)
 Positive = Bounded(lower_bound=-EPSILON)
 Rate = Rate()  # type: ignore
 Date = Date()  # type: ignore
 OptionalDate = OptionalDate()  # type: ignore
+ValDisposition = ValDisposition()
 # # rolling a custom validator for doubling time in case DS wants to add upper bound
 # DoublingTime = OptionalBounded(lower_bound=0-EPSILON, upper_bound=None)
diff --git a/src/penn_chime/validators/validators.py b/src/penn_chime/validators/validators.py
@@ -5,6 +5,15 @@
 
 from .base import Validator
 
+EPSILON = 1.e-7
+
+class OptionalValue(Validator):
+    """Any value at all"""
+    def __init__(self) -> None:
+        pass
+
+    def validate(self, value):
+        pass
 
 class Bounded(Validator):
     """A bounded number."""
@@ -23,6 +32,8 @@ def __init__(
 
     def validate(self, value):
         """This method implicitly validates isinstance(value, (float, int)) because it will throw a TypeError on comparison"""
+        if value is None:
+            raise ValueError(f"This parameter must be set")
         if (self.upper_bound is not None and value > self.upper_bound) \
            or (self.lower_bound is not None and value < self.lower_bound):
             raise ValueError(f"{value} needs to be {self.message[(self.lower_bound, self.upper_bound)]}.")
@@ -47,6 +58,8 @@ def __init__(self) -> None:
         pass
    
     def validate(self, value):
+        if value is None:
+            raise ValueError(f"This parameter must be set")
         if 0 > value or value > 1:
             raise ValueError(f"{value} needs to be a rate (i.e. in [0,1]).")
 
@@ -56,6 +69,8 @@ def __init__(self) -> None:
         pass
 
     def validate(self, value):
+        if value is None:
+            raise ValueError(f"This parameter must be set")
         if not isinstance(value, (date, datetime)):
             raise (ValueError(f"{value} must be a date or datetime object."))
 
@@ -67,3 +82,13 @@ def validate(self, value):
         if value is None:
             return None
         super().validate(value)
+
+class ValDisposition(Validator):
+    def __init__(self) -> None:
+        pass
+
+    def validate(self, value):
+        if value is None:
+            raise ValueError(f"This parameter must be set")
+        Bounded(lower_bound=EPSILON)(value=value.days)
+        Rate()(value=value.rate)
