disease-sh / API

API for Current cases and more stuff about COVID-19 and Influenza
https://disease.sh
GNU General Public License v3.0
2.46k stars 639 forks source link

[Suggestion] Support for null to 0 in government data #690

Closed crazyuploader closed 4 years ago

crazyuploader commented 4 years ago

I don't know about other government end points, but at least on /v2/gov/India there are some null values at the end of the response, it'll save me (and others too) from manually having to check if the value is null or not, I think the default value, i.e. 0 is appropriate enough than to have our app crashing. Thanks in advance!

crazyuploader commented 4 years ago

As for the response, I've had this

{
   "updated": 1589668362672,
   "total": {
      "cases": 85940,
      "recovered": 30153,
      "deaths": 2752,
      "active": 53035
   },
   "states": [
      {
         "state": "Andaman and Nicobar Islands",
         "cases": 33,
         "recovered": 33,
         "deaths": 0,
         "active": 0
      },
      {
         "state": "Andhra Pradesh",
         "cases": 2307,
         "recovered": 1252,
         "deaths": 48,
         "active": 1007
      },
      {
         "state": "Arunachal Pradesh",
         "cases": 1,
         "recovered": 1,
         "deaths": 0,
         "active": 0
      },
      {
         "state": "Assam",
         "cases": 90,
         "recovered": 41,
         "deaths": 2,
         "active": 47
      },
      {
         "state": "Bihar",
         "cases": 1018,
         "recovered": 438,
         "deaths": 7,
         "active": 573
      },
      {
         "state": "Chandigarh",
         "cases": 191,
         "recovered": 37,
         "deaths": 3,
         "active": 151
      },
      {
         "state": "Chhattisgarh",
         "cases": 66,
         "recovered": 56,
         "deaths": 0,
         "active": 10
      },
      {
         "state": "Dadar Nagar Haveli",
         "cases": 1,
         "recovered": 0,
         "deaths": 0,
         "active": 1
      },
      {
         "state": "Delhi",
         "cases": 8895,
         "recovered": 3518,
         "deaths": 123,
         "active": 5254
      },
      {
         "state": "Goa",
         "cases": 15,
         "recovered": 7,
         "deaths": 0,
         "active": 8
      },
      {
         "state": "Gujarat",
         "cases": 9931,
         "recovered": 4035,
         "deaths": 606,
         "active": 5290
      },
      {
         "state": "Haryana",
         "cases": 818,
         "recovered": 439,
         "deaths": 11,
         "active": 368
      },
      {
         "state": "Himachal Pradesh",
         "cases": 76,
         "recovered": 39,
         "deaths": 3,
         "active": 34
      },
      {
         "state": "Jammu and Kashmir",
         "cases": 1013,
         "recovered": 513,
         "deaths": 11,
         "active": 489
      },
      {
         "state": "Jharkhand",
         "cases": 203,
         "recovered": 87,
         "deaths": 3,
         "active": 113
      },
      {
         "state": "Karnataka",
         "cases": 1056,
         "recovered": 480,
         "deaths": 36,
         "active": 540
      },
      {
         "state": "Kerala",
         "cases": 576,
         "recovered": 492,
         "deaths": 4,
         "active": 80
      },
      {
         "state": "Ladakh",
         "cases": 43,
         "recovered": 22,
         "deaths": 0,
         "active": 21
      },
      {
         "state": "Madhya Pradesh",
         "cases": 4595,
         "recovered": 2283,
         "deaths": 239,
         "active": 2073
      },
      {
         "state": "Maharashtra",
         "cases": 29100,
         "recovered": 6564,
         "deaths": 1068,
         "active": 21468
      },
      {
         "state": "Manipur",
         "cases": 3,
         "recovered": 2,
         "deaths": 0,
         "active": 1
      },
      {
         "state": "Meghalaya",
         "cases": 13,
         "recovered": 11,
         "deaths": 1,
         "active": 1
      },
      {
         "state": "Mizoram",
         "cases": 1,
         "recovered": 1,
         "deaths": 0,
         "active": 0
      },
      {
         "state": "Odisha",
         "cases": 672,
         "recovered": 166,
         "deaths": 3,
         "active": 503
      },
      {
         "state": "Puducherry",
         "cases": 13,
         "recovered": 9,
         "deaths": 1,
         "active": 3
      },
      {
         "state": "Punjab",
         "cases": 1935,
         "recovered": 305,
         "deaths": 32,
         "active": 1598
      },
      {
         "state": "Rajasthan",
         "cases": 4727,
         "recovered": 2677,
         "deaths": 125,
         "active": 1925
      },
      {
         "state": "Tamil Nadu",
         "cases": 10108,
         "recovered": 2599,
         "deaths": 71,
         "active": 7438
      },
      {
         "state": "Telengana",
         "cases": 1454,
         "recovered": 959,
         "deaths": 34,
         "active": 461
      },
      {
         "state": "Tripura",
         "cases": 156,
         "recovered": 42,
         "deaths": 0,
         "active": 114
      },
      {
         "state": "Uttarakhand",
         "cases": 82,
         "recovered": 51,
         "deaths": 1,
         "active": 30
      },
      {
         "state": "Uttar Pradesh",
         "cases": 4057,
         "recovered": 2165,
         "deaths": 95,
         "active": 1797
      },
      {
         "state": "West Bengal",
         "cases": 2461,
         "recovered": 829,
         "deaths": 225,
         "active": 1407
      },
      {
         "state": "Cases being reassigned to states",
         "cases": 230,
         "recovered": null,
         "deaths": null,
         "active": null
      }
   ]
}

The source have empty fields like this : -

Screenshot_20200517-081130801

hcovs commented 4 years ago

@crazyuploader Today there was a decent amount for time in which many values were 'null' - prob causing havoc for some usage.

So far it has been assumed that 'null' meant nothing reported and whereas '0' [zero] meant reported but with a 0 value. Unless we are wrong here ... hopefully we can get some insight regarding this point on the API.

crazyuploader commented 4 years ago

@hcovs I understand, but I'm just proposing that having allowNull would make more sense since the app developers can then decide for themselves, which approach do they want to go with, i.e. if they want to consider "null" as zero or not reported. Not having that would only mean app crashes if null not handled, so better we implement that parameter here as well. It'll be up to the app developers whether they want to default "null" values to 0 or not.

pujux commented 4 years ago

@crazyuploader Today there was a decent amount for time in which many values were 'null' - prob causing havoc for some usage.

So far it has been assumed that 'null' meant nothing reported and whereas '0' [zero] meant reported but with a 0 value. Unless we are wrong here ... hopefully we can get some insight regarding this point on the API.

So far (until yesterday GMT), we transformed all values that were NULL to 0 so people wouldn't have to deal with NULL values. After receiving quite a bunch of messages in the last months, we decided to leave NULL as it is. This broke a few applications so we discusses how we could have the feature while being downward compatible. We now added a new query parameter, allowNull, which defaults to false but can be used to get NULL values on properties where we could not receive any data.

This parameter is currently only for v2/all, v2/countries and v2/continents but will be added to v2/states and v2/gov

crazyuploader commented 4 years ago

@puf17640 That would be appreciated, since we as developers can then decide whether we want the value as null or zero.

I actually had thought that the allowNull parameter was applied to every endpoints but trying that on /v2/gov showed me otherwise.

pujux commented 4 years ago

@puf17640 That would be appreciated, since we as developers can then decide whether we want the value as null or zero.

I actually had thought that the allowNull parameter was applied to every endpoints but trying that on /v2/gov showed me otherwise.

yeah it is quite hard to implement it for every endpoint since they all use different data sources, but v2/historical or v2/jhucsse dont return null values as far as i have seen

crazyuploader commented 4 years ago

@puf17640 I would love to open a pull request for the same, but I don't really know the JavaScript used here, so I can't help.

pujux commented 4 years ago

@crazyuploader no problem, I am already working on it

hcovs commented 4 years ago

@puf17640 Thanks for getting back to us here! When do you expect to have the 'allowNull' added to these: v2/states and v2/gov too? -since the code expects 'null' for non-reported and zeros will provide incorrect info.

pujux commented 4 years ago

The code is already in place, I will have to wait for @ebwinters to wake up (he's from the US) to review the PR

hcovs commented 4 years ago

Niice! The API allows the param so I should just work once up. I will check later (I am in the US too.). Thanks!

pujux commented 4 years ago

https://github.com/NovelCOVID/API/pull/691

mymindishazel commented 4 years ago

@hcovs @crazyuploader @puf17640 merged