Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

👍 fixs recovery data in api/v2 jhu #316

Closed
wants to merge 16 commits into from

Conversation

codedawi
Copy link
Contributor

@codedawi codedawi commented May 20, 2020

  • reimplements the get_category("recovered") methods in jhu service.
  • adds/fixes the tests for recovered data now.
  • resolves the merge issue related to missing recovery data from select countries.
    • Sierra Leone
    • Netherlands, Bonaire, Sint Eustatius and Saba
    • Malawi
    • France, Saint Pierre and Miquelon
    • United Kingdom, Falkland Islands (Malvinas)
    • South Sudan
    • Western Sahara
    • Sao Tome and Principe
    • Yemen
    • Comoros
    • Tajikistan
    • Lesotho

While reviewing the jhu service code I uncovered at critical bug that could expose this service to data & uptime issues in the future. In the PR the function parse_history is implemented as a temporary fix which will prevent the service from going down and providing consumers inaccurate data.

Add note to code:

# ***************************************************************************
# TODO: This iteration approach assumes the indexes remain the same
#       and opens us to a CRITICAL ERROR. The removal of a column in the data source
#       would break the API or SHIFT all the data confirmed, deaths, recovery producting
#       incorrect data to consumers.
# ***************************************************************************

Potential address in @Kilo59's refactor mentioned.

FIXES: #161 #276 #113

FIXES: codedawi/covid19-badges#1
Covid-19 Confirmed Covid-19 Deaths Covid-19 Recovered

@codedawi codedawi force-pushed the master branch 2 times, most recently from 6a0bea5 to 761f62d Compare May 20, 2020 05:18
@Kilo59
Copy link
Collaborator

Kilo59 commented May 20, 2020

@codedawi Hey thanks for this! 😄 I'll take a look at this ASAP.

@Kilo59
Copy link
Collaborator

Kilo59 commented May 20, 2020

@codedawi It definitely appears to have fixed the recoveries of many locations (US, AU, etc), but many other locations that are not showing up in the logs as having a "merge error" (DE, CN, CA etc) still show zero recoveries (for me locally).

I'm almost inclined to merge it anyways 😄 .
However certain locations such as the UK are showing partial recovery numbers instead, which IMO is worse than still showing 0.

Any API users please feel free to chime in.

{
  "latest": {
    "confirmed": 250138,
    "deaths": 35422,
    "recovered": 1067
  },
  "locations": [
    {
      "id": 217,
      "country": "United Kingdom",
      "country_code": "GB",
      "country_population": 66488991,
      "province": "Bermuda",
      "last_updated": "2020-05-20T20:52:14.791765Z",
      "coordinates": {
        "latitude": "32.3078",
        "longitude": "-64.7505"
      },
      "latest": {
        "confirmed": 125,
        "deaths": 9,
        "recovered": 78
      }
    },
...
    {
      "id": 223,
      "country": "United Kingdom",
      "country_code": "GB",
      "country_population": 66488991,
      "province": "",
      "last_updated": "2020-05-20T20:52:14.806740Z",
      "coordinates": {
        "latitude": "55.3781",
        "longitude": "-3.436"
      },
      "latest": {
        "confirmed": 248818,
        "deaths": 35341,
        "recovered": 0
      }
    },
...
    {
      "id": 257,
      "country": "United Kingdom",
      "country_code": "GB",
      "country_population": 66488991,
      "province": "Falkland Islands (Malvinas)",
      "last_updated": "2020-05-20T20:52:14.884740Z",
      "coordinates": {
        "latitude": "-51.7963",
        "longitude": "-59.5236"
      },
      "latest": {
        "confirmed": 13,
        "deaths": 0,
        "recovered": 0
      }
    }
  ]
}

I think I would like to merge this immediately to the US hosted version of the API that's based off my own fork.

@Kilo59 Kilo59 self-assigned this May 20, 2020
@Kilo59 Kilo59 added source: jhu help wanted Extra attention is needed labels May 20, 2020
@codedawi
Copy link
Contributor Author

Any update on getting this merged in? If not we can just close.

@codedawi codedawi closed this Dec 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed source: jhu
Projects
None yet
3 participants