Adds units to API response

falquaddoomi commented 5 months ago

This PR alters the fips-value endpoint to include a field named unit, which is either null or is one of the following string values:

"percent": a percentage; the datasets we use encode these as floats between 0 and 1, inclusive
"count": a raw count, represented as an integer, e.g. number of cases
"rate": a rate of occurrence, represented as a float and typically normalized by population
"dollar_amount": raw dollar amount, e.g. income
"rank": a position in a ranking, e.g. for the cancer disparities index ranking of CO counties
"ordinal": a discrete value that can be ordered, e.g. "rising" (1), "falling" (2), "stable" (3)
"categorical": a discrete value that can't be ordered, e.g. "white", "black", "hispanic"
"ratio": range from -1 to +1, where -1 is "least affected" and +1 is "most affected"

Some of these units aren't currently in use, e.g. "categorical", and I suspect many of them can be handled in the same way in the frontend.

The PR includes some simple tests that the unit field is included in the fips-value response and that it matches what's defined in the backend metadata.

The PR also includes a helper class, MeasureMapper, a dict-like that when queried for a key produces a dict of the form {label: <key>, unit: <default_unit>} for measures where the ID and the label are the same, and all the measures have the same unit. A test is included for it as well.

netlify[bot] commented 5 months ago

Deploy Preview for exploring-cancer-in-colorado ready!

Name	Link
Latest commit	d8218e07768643e1577b3a121791521417544782
Latest deploy log	https://app.netlify.com/sites/exploring-cancer-in-colorado/deploys/663e32b311da6f00085a3254
Deploy Preview	https://deploy-preview-67--exploring-cancer-in-colorado.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

vincerubinetti commented 5 months ago

From Teams:

Vince and Faisal - the email on our 'About' page (see below) goes to my email.  Can we change to coe@cuanschutz.edu?  Cydney - does this make sense to you? To suggest a new data source, report an issue, or for general questions or help, please contact us:

also - Cydney and I talked earlier, can we change default for Age strata from '<50' to 'All Ages" - sorry Cydney if you already added an issue for this...

falquaddoomi commented 5 months ago

Vince and Faisal - the email on our 'About' page (see below) goes to my email. Can we change to coe@cuanschutz.edu? Cydney - does this make sense to you?

~Sure, we can sneak that into this PR.~ Actually, I'll just submit a hotfix to main. UPDATE: in as of 4cbb5e646a4fee353a37d2d751eb6b2b28c855eb.

also - Cydney and I talked earlier, can we change default for Age strata from '<50' to 'All Ages" - sorry Cydney if you already added an issue for this...

Cydney did in fact create an issue for this one, https://github.com/colorado-cancer-center/ecco/issues/66. I responded to it, and FWIW the reason it's occurring is because the API sends back factors in database order and "<50" just happens to come before "All Ages". A quick fix could be to sort the values in such a way that the "All" option comes up first, but the fact that the values are an object, not a list, means that the sort order may not be respected in the frontend and/or backend. Since there's already an issue and we might need to discuss the fix, I'll submit this as a separate PR that might require frontend changes as well.

vincerubinetti commented 5 months ago

Cydney did in fact create an issue for this one, https://github.com/colorado-cancer-center/ecco/issues/66. I responded to it, and FWIW the reason it's occurring is because the API sends back factors in database order and "<50" just happens to come before "All Ages". A quick fix could be to sort the values in such a way that the "All" option comes up first, but the fact that the values are an object, not a list, means that the sort order may not be respected in the frontend and/or backend. Since there's already an issue and we might need to discuss the fix, I'll submit this as a separate PR that might require frontend changes as well.

Actually I think this is just a frontend bug, and doesn't need any backend change (unless you think factors need to be displayed on the frontend in an order other than alphabetical). Looking at the frontend code, it's not even using the default you've hard-coded into the backend. I'm pretty sure I was using this at one point in the past (or am I remembering a default for levels/cats/measures...), but I guess it got lost in the several refactors/bug-fixes of the frontend factors code.

I'll address it.

Edit: Now I seem to remember you took the default field out from the backend response, can't remember why. I added back in what you had to return the default in the /measures endpoint. Also found a case where the default was not in the set of value options: cancer mortality, childhood, age. In that case, frontend falls back to first option. I committed it to this PR to save my work, but we can revert it tomorrow if you want it in a separate PR.

falquaddoomi commented 5 months ago

Regarding the default: let's just keep it in here, since I've already made commits after yours. It's possible to extract it, but it's a little messy to sync up your local branch if one of us rewrites history, and it's hard to justify for one commit.

FYI, I changed "ratio" to "least_most" in the backend; I hope it's ok, but I updated the frontend as well in the two places where you were checking for "ratio".

colorado-cancer-center / ecco