Repository: CodeForPhilly/chime (python)

Issue:
[dash] Add log scale checkbox in sidebar
Put it at the bottom of the sidebar. When checked switch the y-axis scale to log scale. Equivalent to [`altair.ScaleType`](https://altair-viz.github.io/user_guide/generated/core/altair.ScaleType.html#altair.ScaleType)`== 'log'`

Pull request:
#353 Adds Log Scale Checkbox in Sidebar
<!-- Please read the contributing guidelines before submitting: https://codeforphilly.github.io/chime/CONTRIBUTING.html -->

<!--
- If this PR affects devops or model, start your PR title with `[DevOps]` or `[Model]`
- Update the user or developer documentation, if affected.
-->

# Link to issue

- Fixes #353 

# Changelog entry

- Added a checkbox in the sidebar to toggle between log scale and linear scale on all charts.
- Updated chart creation code to toggle between log scale and linear scale. Some adjustments to the input data to the chart were needed, because a y value of zero on the chart when using log scale causes it to glitch horribly.
- Updated parameters so that `--use-log-scale` can be passed in from the command line. 

# Additional info

- I noticed some missing and unclear documentation, so I added doc tags where appropriate. I'm sorry if this made the pull request bigger than expected. Just trying to keep things tidy!

---

## <center>Pull Request Checklist</center>

### <center>For Reviewer</center>

- [X] Submitted under the correct tag
- [ ] Runs without error or warning
   - There are still JavaScript warnings in the console, but they appear to be happening on the develop branch as well. Should I be worried?
- [ ] Documentation  revised / included
   - Does anyone know how I would update the user documentation?
- [X] Unit tests included
- [X] Issues referenced
- [X] Does not create new bugs / issues


Patch:
diff --git a/src/penn_chime/model/parameters.py b/src/penn_chime/model/parameters.py
index 72f39e1..b15d559 100644
--- a/src/penn_chime/model/parameters.py
+++ b/src/penn_chime/model/parameters.py
@@ -137,6 +137,7 @@ VALIDATORS = {
     "ventilated": ValDisposition,
     "hospitalized": ValDisposition,
     "icu": ValDisposition,
+    "use_log_scale": OptionalValue
 }
 
 
@@ -161,6 +162,7 @@ HELP = {
     "relative_contact_rate": "Social distancing reduction rate: 0.0 - 1.0",
     "ventilated_days": "Average days on ventilator",
     "ventilated_rate": "Ventilated Rate: 0.0 - 1.0",
+    "use_log_scale": "Flag to use logarithmic scale on charts instead of linear scale."
 }
 
 
@@ -168,9 +170,9 @@ ARGS = (
     (
         "parameters",
         str,
-        None,
-        None,
-        False,
+        None, # Min value
+        None, # Max value
+        False, # Whether it is required or optional.
     ),
     (
         "current_hospitalized",
@@ -298,15 +300,24 @@ ARGS = (
         1.0,
         True,
     ),
+    (
+        "use_log_scale",
+        bool,
+        None,
+        None,
+        False
+    )
 )
 
 
 def to_cli(name):
     return "--" + name.replace('_', '-')
 
-
 class Parameters:
-    """Parameters."""
+    """
+    Object containing all of the parameters that can be adjusted by the user, either from the command line or using
+    the side bar of the web app.
+    """
 
     @classmethod
     def parser(cls):
@@ -315,11 +326,20 @@ class Parameters:
 
         for name, cast, min_value, max_value, required in ARGS:
             arg = to_cli(name)
-            parser.add_argument(
-                arg,
-                type=validator(arg, cast, min_value, max_value, required),
-                help=HELP.get(name),
-            )
+            if cast == bool:
+                # This argument is a command-line flag and does not need validation.
+                parser.add_argument(
+                    arg,
+                    action='store_true',
+                    help=HELP.get(name),
+                )
+            else:
+                # Use a custom validator for any arguments that take in values.
+                parser.add_argument(
+                    arg,
+                    type=validator(arg, cast, min_value, max_value, required),
+                    help=HELP.get(name),
+                )
         return parser
 
     @classmethod
@@ -396,6 +416,7 @@ class Parameters:
         self.relative_contact_rate = None
         self.recovered = None
         self.ventilated = None
+        self.use_log_scale = False
 
         passed_and_default_parameters = {}
         for key, value in kwargs.items():
diff --git a/src/penn_chime/view/charts.py b/src/penn_chime/view/charts.py
index 394f226..bb4eca2 100644
--- a/src/penn_chime/view/charts.py
+++ b/src/penn_chime/view/charts.py
@@ -1,6 +1,6 @@
 from typing import Dict, Optional
 
-from altair import Chart
+from altair import Chart, Scale
 import pandas as pd
 import i18n
 import numpy as np
@@ -9,12 +9,24 @@ from ..constants import DATE_FORMAT
 
 
 def build_admits_chart(
-    *, alt, admits_floor_df: pd.DataFrame, max_y_axis: Optional[int] = None
+    *, alt, admits_floor_df: pd.DataFrame, max_y_axis: Optional[int] = None, use_log_scale: bool = False
 ) -> Chart:
-    """Build admits chart."""
-    y_scale = alt.Scale()
-    if max_y_axis is not None:
-        y_scale.domain = (0, max_y_axis)
+    """
+    This builds the "New Admissions" chart, projecting daily admissions over time.
+
+    Args:
+        alt: Reference to Altair package.
+        admits_floor_df: Pandas data frame containing three columns: "admits_hospitalized", "admits_icu", and
+        "admits_ventilated".
+        max_y_axis: Optional maximum value for the Y axis of the chart.
+        use_log_scale: Set to true to use a logarithmic scale on the Y axis. Default is linear scale.
+
+    Returns: The newly created chart.
+
+    """
+
+    adjusted_admits_floor_df = __adjust_data_for_log_scale(admits_floor_df) if use_log_scale else admits_floor_df
+    y_scale = __build_y_scale(alt, max_y_axis, use_log_scale)
 
     x = dict(shorthand="date:T", title=i18n.t("charts-date"), axis=alt.Axis(format=(DATE_FORMAT)))
     y = dict(shorthand="value:Q", title=i18n.t("charts-daily-admissions"), scale=y_scale)
@@ -40,7 +52,7 @@ def build_admits_chart(
         .transform_filter(alt.datum.day == 0)
         .mark_rule(color="black", opacity=0.35, size=2)
     )
-    admits_floor_df_renamed = admits_floor_df.rename({
+    admits_floor_df_renamed = adjusted_admits_floor_df.rename({
         "admits_hospitalized": i18n.t("admits_hospitalized"),
         "admits_icu": i18n.t("admits_icu"),
         "admits_ventilated": i18n.t("admits_ventilated")
@@ -53,12 +65,24 @@ def build_admits_chart(
 
 
 def build_census_chart(
-    *, alt, census_floor_df: pd.DataFrame, max_y_axis: Optional[int] = None
+    *, alt, census_floor_df: pd.DataFrame, max_y_axis: Optional[int] = None, use_log_scale: bool = False
 ) -> Chart:
-    """Build census chart."""
-    y_scale = alt.Scale()
-    if max_y_axis:
-        y_scale.domain = (0, max_y_axis)
+    """
+    This builds the "Admitted Patients" census chart, projecting total number of patients in the hospital over time.
+
+    Args:
+        alt: Reference to Altair package.
+        census_floor_df: Pandas data frame containing three columns: "census_hospitalized", "census_icu", and
+        "census_ventilated".
+        max_y_axis: Optional maximum value for the Y axis of the chart.
+        use_log_scale: Set to true to use a logarithmic scale on the Y axis. Default is linear scale.
+
+    Returns: The newly created chart.
+
+    """
+
+    adjusted_census_floor_df = __adjust_data_for_log_scale(census_floor_df) if use_log_scale else census_floor_df
+    y_scale = __build_y_scale(alt, max_y_axis, use_log_scale)
 
     x = dict(shorthand="date:T", title=i18n.t("charts-date"), axis=alt.Axis(format=(DATE_FORMAT)))
     y = dict(shorthand="value:Q", title=i18n.t("charts-census"), scale=y_scale)
@@ -84,7 +108,7 @@ def build_census_chart(
         .transform_filter(alt.datum.day == 0)
         .mark_rule(color="black", opacity=0.35, size=2)
     )
-    census_floor_df_renamed = census_floor_df.rename({
+    census_floor_df_renamed = adjusted_census_floor_df.rename({
         "census_hospitalized": i18n.t("census_hospitalized"),
         "census_icu": i18n.t("census_icu"),
         "census_ventilated": i18n.t("census_ventilated")
@@ -97,12 +121,24 @@ def build_census_chart(
 
 
 def build_sim_sir_w_date_chart(
-    *, alt, sim_sir_w_date_floor_df: pd.DataFrame, max_y_axis: Optional[int] = None
+    *, alt, sim_sir_w_date_floor_df: pd.DataFrame, max_y_axis: Optional[int] = None, use_log_scale: bool = False
 ) -> Chart:
-    """Build sim sir w date chart."""
-    y_scale = alt.Scale()
-    if max_y_axis is not None:
-        y_scale.domain = (0, max_y_axis)
+    """
+    This builds the "Susceptible, Infected, and Recovered" chart, projecting the number of those individuals in the
+    hospital's region over time.
+
+    Args:
+        alt: Reference to the Altair package.
+        sim_sir_w_date_floor_df: A Pandas data frame with columns named "susceptible", "infected", and "recovered".
+        max_y_axis: Optional maximum value for the Y axis of the chart.
+        use_log_scale: Set to true to use a logarithmic scale on the Y axis. Default is linear scale.
+
+    Returns: The newly created chart.
+
+    """
+
+    adjusted_sim_sir_w_date_floor_df = __adjust_data_for_log_scale(sim_sir_w_date_floor_df) if use_log_scale else sim_sir_w_date_floor_df
+    y_scale = __build_y_scale(alt, max_y_axis, use_log_scale)
 
     x = dict(shorthand="date:T", title=i18n.t("charts-date"), axis=alt.Axis(format=(DATE_FORMAT)))
     y = dict(shorthand="value:Q", title=i18n.t("charts-count"), scale=y_scale)
@@ -128,7 +164,7 @@ def build_sim_sir_w_date_chart(
         .transform_filter(alt.datum.day == 0)
         .mark_rule(color="black", opacity=0.35, size=2)
     )
-    sim_sir_w_date_floor_df_renamed = sim_sir_w_date_floor_df.rename({
+    sim_sir_w_date_floor_df_renamed = adjusted_sim_sir_w_date_floor_df.rename({
         "susceptible": i18n.t("susceptible"),
         "infected": i18n.t("infected"),
         "recovered": i18n.t("recovered")
@@ -146,3 +182,37 @@ def build_table(
     table_df.date = table_df.date.dt.strftime(DATE_FORMAT)
     table_df_renamed = table_df.rename(labels, axis=1)
     return table_df_renamed
+
+
+def __adjust_data_for_log_scale(dataframe: pd.DataFrame) -> pd.DataFrame:
+    """
+    This will clean and adjust some of the data so that Altair can plot it using a logarithmic scale. Altair does not
+    allow zero values on the Y axis when plotting with a logarithmic scale, as log(0) is undefined.
+
+    Args:
+        dataframe: The data to plot on the chart.
+
+    Returns: A new data frame with the appropriate adjustments for plotting on a log scale.
+
+    """
+    return dataframe.replace(0, float('nan'))  # We use NaN so that the values will not appear at all on the chart.
+
+
+def __build_y_scale(alt, max_y_axis: Optional[int] = None, use_log_scale: bool = False) -> Scale:
+    """
+    Creates the Y axis of the chart, taking into account some of the configuration parameters set by the user.
+
+    Args:
+        alt: Reference to Altair package.
+        max_y_axis: The maximum value of the Y axis. This is optional.
+        use_log_scale: Whether to use a logarithmic scale instead of a linear scale.
+
+    Returns: A newly created Scale instance.
+
+    """
+    scale_type = 'log' if use_log_scale else 'linear'
+    y_scale = alt.Scale(type=scale_type)
+    if max_y_axis is not None:
+        y_scale.domain = (0, max_y_axis)
+
+    return y_scale
diff --git a/src/penn_chime/view/st_app.py b/src/penn_chime/view/st_app.py
index 26811d2..eade331 100644
--- a/src/penn_chime/view/st_app.py
+++ b/src/penn_chime/view/st_app.py
@@ -49,7 +49,8 @@ def main():
 
     st.subheader(i18n.t("app-new-admissions-title"))
     st.markdown(i18n.t("app-new-admissions-text"))
-    admits_chart = build_admits_chart(alt=alt, admits_floor_df=m.admits_floor_df, max_y_axis=p.max_y_axis)
+    admits_chart = build_admits_chart(alt=alt, admits_floor_df=m.admits_floor_df, max_y_axis=p.max_y_axis, use_log_scale=p.use_log_scale)
+
     st.altair_chart(admits_chart, use_container_width=True)
     display_download_link(
         st,
@@ -60,7 +61,8 @@ def main():
 
     st.subheader(i18n.t("app-admitted-patients-title"))
     st.markdown(i18n.t("app-admitted-patients-text"))
-    census_chart = build_census_chart(alt=alt, census_floor_df=m.census_floor_df, max_y_axis=p.max_y_axis)
+    census_chart = build_census_chart(alt=alt, census_floor_df=m.census_floor_df, max_y_axis=p.max_y_axis, use_log_scale=p.use_log_scale)
+
     st.altair_chart(census_chart, use_container_width=True)
     display_download_link(
         st,
@@ -93,7 +95,8 @@ def main():
 
     st.subheader(i18n.t("app-SIR-title"))
     st.markdown(i18n.t("app-SIR-text"))
-    sim_sir_w_date_chart = build_sim_sir_w_date_chart(alt=alt, sim_sir_w_date_floor_df=m.sim_sir_w_date_floor_df)
+    sim_sir_w_date_chart = build_sim_sir_w_date_chart(alt=alt, sim_sir_w_date_floor_df=m.sim_sir_w_date_floor_df, use_log_scale=p.use_log_scale)
+
     st.altair_chart(sim_sir_w_date_chart, use_container_width=True)
     display_download_link(
         st,
diff --git a/src/penn_chime/view/st_display.py b/src/penn_chime/view/st_display.py
index 86a734f..6f5a1ea 100644
--- a/src/penn_chime/view/st_display.py
+++ b/src/penn_chime/view/st_display.py
@@ -166,10 +166,10 @@ class CheckboxInput(Input):
 
 
 def display_sidebar(st, d: Parameters) -> Parameters:
-    # Initialize variables
-    # these functions create input elements and bind the values they are set to
-    # to the variables they are set equal to
-    # it's kindof like ember or angular if you are familiar with those
+    """
+    Initializes the UI in the sidebar. These function calls create input elements, and bind the values they are set to
+    to the appropriate variables. It's similar to Ember or Angular, if you are familiar with those frameworks.
+    """
 
     st_obj = st.sidebar
     # used_widget_key = st.get_last_used_widget_key ( )
@@ -361,7 +361,9 @@ def display_sidebar(st, d: Parameters) -> Parameters:
         max_y_axis = max_y_axis_input()
 
     current_date = current_date_input()
-    #Subscribe implementation
+    use_log_scale = st.sidebar.checkbox(label="Use logarithmic scale on charts instead of linear scale.", value=d.use_log_scale)
+
+    # Subscribe implementation
     subscribe(st_obj)
 
     return Parameters(
@@ -386,9 +388,10 @@ def display_sidebar(st, d: Parameters) -> Parameters:
         ventilated=Disposition.create(
             rate=ventilated_rate,
             days=ventilated_days),
+        use_log_scale=use_log_scale
     )
 
-#Read the environment variables and cteate json key object to use with ServiceAccountCredentials
+# Read the environment variables and create json key object to use with ServiceAccountCredentials
 def readGoogleApiSecrets():
     client_secret = {}
     os.getenv


Test patch:
diff --git a/tests/penn_chime/view/test_charts.py b/tests/penn_chime/view/test_charts.py
index ce96e88..7419353 100644
--- a/tests/penn_chime/view/test_charts.py
+++ b/tests/penn_chime/view/test_charts.py
@@ -1,5 +1,6 @@
 import altair as alt
 import pytest
+import math
 import os
 import i18n
 
@@ -15,10 +16,18 @@ from penn_chime.view.charts import (
 
 DISPOSITION_KEYS = ("hospitalized", "icu", "ventilated")
 
+# These are the localized column names for the dataframe sent to the charting library.
+admits_icu_key = i18n.t("admits_icu")
+admits_hospitalized_key = i18n.t("admits_hospitalized")
+admits_ventilated_key = i18n.t("admits_ventilated")
+census_icu_key = i18n.t("census_icu")
+census_hospitalized_key = i18n.t("census_hospitalized")
+census_ventilated_key = i18n.t("census_ventilated")
+
 def test_admits_chart(admits_floor_df):
     chart = build_admits_chart(alt=alt, admits_floor_df=admits_floor_df)
     assert isinstance(chart, (alt.Chart, alt.LayerChart))
-    assert round(chart.data.iloc[40][i18n.t("admits_icu")], 0) == 38
+    assert round(chart.data.iloc[40][admits_icu_key], 0) == 38
 
     # test fx call with no params
     with pytest.raises(TypeError):
@@ -28,9 +37,60 @@ def test_admits_chart(admits_floor_df):
 def test_census_chart(census_floor_df):
     chart = build_census_chart(alt=alt, census_floor_df=census_floor_df)
     assert isinstance(chart, (alt.Chart, alt.LayerChart))
-    assert chart.data.iloc[1][i18n.t("census_hospitalized")] == 3
-    assert chart.data.iloc[49][i18n.t("census_ventilated")] == 365
+    assert chart.data.iloc[1][census_hospitalized_key] == 3
+    assert chart.data.iloc[49][census_ventilated_key] == 365
 
     # test fx call with no params
     with pytest.raises(TypeError):
         build_census_chart()
+
+def test_admits_chart_log_scale(admits_floor_df):
+    """
+    Verifies that if the log scale is used, then the values on the chart are adjusted appropriately.
+
+    Args:
+        admits_floor_df: Sample admission data.
+
+    """
+    chart = build_admits_chart(alt=alt, admits_floor_df=admits_floor_df, use_log_scale=True)
+
+    # We check a few values to verify that zero was replaced with NaN.
+    assert chart.data.iloc[1][admits_hospitalized_key] == 2
+    assert math.isnan(chart.data.iloc[1][admits_icu_key])
+    assert math.isnan(chart.data.iloc[1][admits_ventilated_key])
+
+    assert chart.data.iloc[2][admits_hospitalized_key] == 2
+    assert math.isnan(chart.data.iloc[2][admits_icu_key])
+    assert math.isnan(chart.data.iloc[2][admits_ventilated_key])
+
+    assert chart.data.iloc[3][admits_hospitalized_key] == 3
+    assert math.isnan(chart.data.iloc[3][admits_icu_key])
+    assert math.isnan(chart.data.iloc[3][admits_ventilated_key])
+
+    assert chart.data.iloc[4][admits_hospitalized_key] == 3
+    assert chart.data.iloc[4][admits_icu_key] == 1
+    assert math.isnan(chart.data.iloc[4][admits_ventilated_key])
+
+def test_census_chart_log_scale(census_floor_df):
+    """
+    Verifies that if the log scale is used, then the values on the chart are adjusted appropriately.
+
+    Args:
+        census_floor_df: Sample census data.
+
+    """
+    chart = build_census_chart(alt=alt, census_floor_df=census_floor_df, use_log_scale=True)
+
+    # We check a few values to verify that zero was replaced with NaN.
+    assert math.isnan(chart.data.iloc[0][census_hospitalized_key])
+    assert math.isnan(chart.data.iloc[0][census_icu_key])
+    assert math.isnan(chart.data.iloc[0][census_ventilated_key])
+
+    assert chart.data.iloc[1][census_hospitalized_key] == 3
+    assert chart.data.iloc[1][census_icu_key] == 1
+    assert chart.data.iloc[1][census_ventilated_key] == 1
+
+    assert chart.data.iloc[2][census_hospitalized_key] == 6
+    assert chart.data.iloc[2][census_icu_key] == 2
+    assert chart.data.iloc[2][census_ventilated_key] == 2
+