act-now-coalition / covid-data-model

Data backend providing computed data for the graphs displayed at https://covidactnow.org
https://covidactnow.org/
MIT License
149 stars 57 forks source link

Round metric values. #1138

Closed mikelehen closed 3 years ago

mikelehen commented 3 years ago

This fixes the Arkansas issue we looked at this morning (as well as a number of other cases where states/counties were on a metric threshold).

I verified that the generated summary file on the frontend (where we already were doing this kind of rounding) was unaffected except:

  1. As desired, risk levels were fixed when a rounded metric value ended up on a metric threshold.
  2. JavaScript and Python round slightly differently when the value ends in a 5 (e.g. 0.115 rounds to 0.12 in JavaScript but 0.11 in Python).

Also worth noting that the resulting .timeseries.json API endpoints are ~35% smaller.

mikelehen commented 3 years ago

Note the failing tests are unrelated to my change. One of the tests has been crashing (only in CI) for the last few days. Last time this happened, Tom disabled parallel tests (https://github.com/covid-projections/covid-data-model/blob/164d80feee77ca2e771ca1d78d27b1677c7d1e55/Makefile#L7) but the problem seems to have come back. 😬

mikelehen commented 3 years ago

Location page color may not match summary file?

mikelehen commented 3 years ago

More Rigorous Sig Figs in the API

ghop02 commented 3 years ago

Note the failing tests are unrelated to my change. One of the tests has been crashing (only in CI) for the last few days. Last time this happened, Tom disabled parallel tests (

https://github.com/covid-projections/covid-data-model/blob/164d80feee77ca2e771ca1d78d27b1677c7d1e55/Makefile#L7

) but the problem seems to have come back. 😬

Ah sad.

Does skipping those two tests that are failing let the tests pass? If they do - we could skip them in pytest and make a ticket for fixing those..

But not too worried about it now

mikelehen commented 3 years ago

I'll try disabling the test and see...

I was also wondering about removing pytest-xdist to see if that solves the crash (or makes it more visible so we know what's going on)...