feat: add disease prevalence rates chart by suhana13 · Pull Request #1834 · datacommonsorg/website

suhana13 · 2022-11-14T16:46:25Z

In this PR, I'm re-using the maptile component from the graph explorer to plot the disease prevalence rates for the disease browser. Here is a screenshot for one of the diseases.

beets

As you make updates to map tile, please ensure that the topic pages still work:
/topic/equity/country/USA

At a high level, also wondering if we need a denominator for these maps since we are calling them prevalence charts.

Thanks!

beets · 2022-11-16T21:29:52Z

+export const USA: NamedTypedPlace = {
+  dcid: "country/USA",
+  name: "United States of America",
+  types: ["Country"],
+};


please add this to constants.ts

beets · 2022-11-16T21:32:33Z

 import { ChartTileContainer } from "./chart_tile";
 import { CHART_HEIGHT } from "./constants";
 import { ReplacementStrings } from "./string_utils";
+export const MAP_ID = "map";


The topic page is designed for multiple map tiles to load, which is why the element ID is passed in as a prop, and not hardcoded to a single ID (HTML element IDs must be unique within a page).

Please pass this value in from the disease page component.

beets · 2022-11-16T21:36:08Z

    }
    return place.name + ": " + value;
  };
+  document.getElementById(MAP_ID).innerHTML = "";


This should not be required. Maybe this was added after the map Id's were hardcoded?

beets · 2022-11-16T21:36:16Z

+  document.getElementById(MAP_ID).innerHTML = "";
  drawD3Map(
-    props.id,
+    MAP_ID,


Please revert.

beets · 2022-11-16T21:43:37Z

+      "Count_MedicalConditionIncident_Condition" + this.props.dcid;
+    const statVarDisease = {} as StatVarSpec;
+    statVarDisease.statVar = diseasePrevalenceStatVarValue;
+    statVarDisease.denom = "";


Since we are showing "prevalence", should this be Count_Person? That might also negate the changes in map_tile when there is no denominator defined.

+1 - showing just the count of a disease might not be super meaningful because that count can be impacted by the population of a certain area

Displaying percentages instead of counts! Thanks for the great suggestion!

beets · 2022-11-16T21:45:39Z

+      if (
+        !_.isEmpty(populationData.data) &&
+        denomStatVar in populationData.data
+      ) {
+        population = populationData.data[denomStatVar];
+      }


perhaps, this should check if denomStatVar is defined in the first place. checking for "" in object works but is error prone.

also see comment above about always using a denominator for this..

chejennifer

nice job getting the map to work!

chejennifer · 2022-11-30T02:37:41Z

-
+    const diseasePrevalenceStatVarValue =
+      "Count_MedicalConditionIncident_Condition" + this.props.dcid;
+    const statVarDisease = {} as StatVarSpec;


instead of defining an empty object and then setting the fields, you could just define the object with all the values like this:

const statVarSpec = { statVar: diseasePrevalenceStatVarValue, denom: "", ... }

chejennifer · 2022-11-30T02:39:01Z

      { id: "drugSource", name: "Drug Source" },
    ];
-
+    const diseasePrevalenceStatVarValue =


nit: since this is the stat var dcid, maybe diseasePrevalenceStatVarDcid instead

chejennifer · 2022-11-30T02:40:43Z

  name: "Earth",
  types: ["Planet"],
 };
+export const USA: NamedTypedPlace = {


for consistency with the other NamedTypedPlace constants, let's name this "USA_NAMED_TYPED_PLACE"

Changed the name to USA_NAMED_TYPED_PLACE, for consistency!

chejennifer · 2022-11-30T02:47:59Z

+      "Count_MedicalConditionIncident_Condition" + this.props.dcid;
+    const statVarDisease = {} as StatVarSpec;
+    statVarDisease.statVar = diseasePrevalenceStatVarValue;
+    statVarDisease.denom = "";


+1 - showing just the count of a disease might not be super meaningful because that count can be impacted by the population of a certain area

@suhana13

also fixed topic page maps without denominator (from #1834, thanks @suhana13) given time pressure, i'd be happy to address comments in a followup pr

beets

one small nit left and feel free to submit after fixing it

beets · 2022-12-09T20:59:10Z

    }
    return place.name + ": " + value;
  };
+  document.getElementById(props.id).innerHTML = "";


nit: this shouldn't be required anymore -- please remove.

@beets , the reason I added this was because before it, the map was getting duplicated multiple times (every time the state of the data got updated) but with the addition of this line, it's only being plotted once:)

I see - let's do this within the disease page then (not in map_tile). It looks like we'll have to look at how the page is being redrawn with data refreshes.

chejennifer

thanks for the changes!

chejennifer · 2022-12-09T22:19:35Z

-          100,
-        props.statVarSpec.unit
-      );
+      // shows upto 4 decimal points for very low values


maybe we can use toPrecision & keep 2 significant digits for values whose absolute value is between 0-1.

chejennifer

thanks for the changes

chejennifer · 2022-12-12T15:55:35Z

-        props.statVarSpec.unit
-      );
+      // shows upto 4 decimal points for very low values
+      if (chartData.dataValues[place.dcid] < 0.01) {


let's do this for all values where the absolute value (in case there are negatives) is between 0 and 1

That's a great catch! Changed it to absolute value.

please also change the check so that this case is only for numbers between 0 and 1

chejennifer · 2022-12-12T15:59:54Z

-      );
+      // shows upto 4 decimal points for very low values
+      if (chartData.dataValues[place.dcid] < 0.01) {
+        const val = Number(chartData.dataValues[place.dcid]);


nit: don't need the Number() since dataValues values are already numbers
nit: let's be more specific in the naming about what value this variable is referring to because we already have another variable named value.

Done! Though I still had to do Number(chartDatavalue.toPrecision(2)) in line 286, as .Precision() returned a string and we want a number!

chejennifer

Thanks for all the changes, please fix this nit before merging

chejennifer · 2022-12-13T15:35:35Z

+        Math.abs(chartData.dataValues[place.dcid]) < 1 &&
+        Math.abs(chartData.dataValues[place.dcid]) > 0
+      ) {
+        const chartDatavalue = chartData.dataValues[place.dcid];


nit: chartDataValue instead of chartDatavalue
nit: this variable can also be used in line 284, 285 & 295, so let's pull it out to before it's used by all those cases

feat: add disease prevalence rates chart

086492c

suhana13 requested review from beets, chejennifer and shifucun November 14, 2022 16:46

beets reviewed Nov 16, 2022

View reviewed changes

Suhana Bedi added 2 commits November 21, 2022 10:26

feat:add USA to constants.ts

dfe0761

revert map_id to props.id

6e6a527

chejennifer reviewed Nov 30, 2022

View reviewed changes

beets mentioned this pull request Dec 2, 2022

add equity page for california #1853

Merged

beets added a commit that referenced this pull request Dec 2, 2022

add equity page for california (#1853)

f146699

also fixed topic page maps without denominator (from #1834, thanks @suhana13) given time pressure, i'd be happy to address comments in a followup pr

Suhana Bedi added 2 commits December 9, 2022 12:09

feat: implement PR nits

3912f98

merge master into disease-map

df70aff

beets approved these changes Dec 9, 2022

View reviewed changes

feat: implement PR nits

fc0fba4

chejennifer reviewed Dec 9, 2022

View reviewed changes

Suhana Bedi added 2 commits December 9, 2022 17:24

fix: change from decimal to precision

d228ecd

feat: render graph once state is updated

7f54048

chejennifer reviewed Dec 12, 2022

View reviewed changes

Suhana Bedi added 4 commits December 12, 2022 12:37

implement PR nits

4f0f747

Merge branch 'master' into disease-map

0799a7e

feat: add data for maps

e09e179

fix: absolute value condition

0ca0e59

chejennifer approved these changes Dec 13, 2022

View reviewed changes

Suhana Bedi and others added 7 commits December 17, 2022 09:45

Merge branch 'master' into disease-map

839e890

change map_tile file path

94ebef9

Merge branch 'master' into disease-map

890c4ff

Merge branch 'master' into disease-map

d4a836a

Merge branch 'master' into disease-map

b5c0cd5

Merge branch 'master' into disease-map

bf7bd37

Merge branch 'master' into disease-map

df35cda

feat: add chart height

a354208

suhana13 merged commit 432c5be into datacommonsorg:master Jan 9, 2023

rohitkumarbhagat mentioned this pull request Apr 21, 2026

2026-04-21 Custom DC stable release #6227

Merged

Conversation

suhana13 commented Nov 14, 2022

Uh oh!

beets left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chejennifer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

beets left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chejennifer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chejennifer left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chejennifer left a comment

chejennifer left a comment •

edited

Loading