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

Nat dogfooding notes #212

Closed
23 of 42 tasks
nmdefries opened this issue Nov 8, 2023 · 2 comments
Closed
23 of 42 tasks

Nat dogfooding notes #212

nmdefries opened this issue Nov 8, 2023 · 2 comments
Assignees

Comments

@nmdefries
Copy link
Contributor

nmdefries commented Nov 8, 2023

  • Documentation website
    • the epidatr vignette isn't up to date about ways to set api keys -- it doesn't mention the set_api_key fn. code is up to date, but site needs to be released (looks like releases not done automatically(
      • "signal discovery" how-to should be more fleshed out and NOT listed as advanced or experimental. Potentially better as a stand-alone vignette
      • Can copy over some covidcast documentation, for both signal discovery and other
      • geo discovery/how to find geo ids (basic version done)
      • how to use metadata
      • versioned data (separate vignette)
      • getting the call obj shouldn't be part of "getting started"
      • And then it just has a huge list of endpoints?? doesn't seem super useful since it doesn't actually describe them, just link to the online docs and provide the same example from each fn doc page
    • landing page looks more like "getting started". Should focus more on what the package is/provides, how it differs from covidcast, setup, links to other resources, good vignettes to start with, etc
    • Reference page should have more info about the different fn sections #238
      • private endpoints should be at the bottom (or maybe even hidden/collapsed by default) since they're not relevant to most users.
  • Function documentation (most of these are really issues with the Epidata doc website)
  • Endpoint standardization (moved to Standardize endpoint arg names and formats #242)
    • Integer-type geo codes (e.g. fips) should be able to be passed as integers to geo_values, like dates can
    • make pub_delphi support a range of epiweeks; currently only takes one epiweek date at a time. This could also be changed in delphi-epidata
    • All endpoints should accept wildcard for dates
      • Want a fn that converts any "*" passed for dates into a long date period
    • don't like that endpoints don't all have the same args (although I can see why). Might be nice to unify them more, though. e.g. pub_ecdc_ili uses epiweeks when it could just use time_values
    • issues doesn't support a list of dates
    • pub_covid_hosp_facility says it takes weeks, but actually takes days corresponding to weeks
  • Naming (moved to Make endpoint names more descriptive #241)
    • pub_covid_hosp_facility_lookup arg fips_code name doesn't parallel arg zip. Neither should have "code" in their names.
    • pub_gft name too short and not descriptive. Ditto pvt_ght, and pvt_cdc and pub_delphi (although for the last two it's more that "CDC" and "Delphi" are very broad)
    • name covidcast_epidata is not informative. What about "covidcast_signal_explorer"?
  • Other
    • In message "source, signals, time_type, geo_type, time_values, and geo_value are all required", geo_value should be plural.
      • Also it would be nice if the message said specifically what arg/s you were missing
    • Data problem: lots of -999999 in pub_covid_hosp_facility total_adult_patients_hospitalized_confirmed_covid_7_day_avg field, but also has NAs. Is resolving this within our purview? Or are we just re-reporting this as-is? moved to delphi-epidata
    • Endpoints should use defaults so that users are able to get some data specifying no or minimal args. This mirrors covidcast behavior.
      • endpoints should default to providing data for all dates
      • geo_type should have no default
    • pub_covid_hosp_facility (and presumably all other endpoints) forces me to specify a time range. Why not just default to giving me data for all dates?
    • Render markdown in covidcast_epidata output #239

[1] "Information about hospital admissions, provided to us by health system partners. Using inpatient claim counts, we estimate the percentage of new hospital admissions with a COVID-associated diagnosis code in a given location, on a given day.\n\nSee also our [Health & Human Services](https://cmu-delphi.github.io/delphi-epidata/api/covidcast-signals/hhs.html\) data source for official COVID hospitalization reporting from the US Department of Health & Human Services."

@brookslogan
Copy link
Contributor

brookslogan commented Nov 21, 2023

A lot of great points! Couple of notes

unclear what pub_covid_hosp_facility signal cols represent. There are covariance columns (names end with "_cov")??

_cov = "coverage", not "covariance". And "coverage" in some sense of % facilities reporting the corresponding non-_cov column or something like that. Would need to read upstream docs. I think they do just use that _cov naming upstream though, unfortunate.

Should endpoints use defaults so that users are able to get some data? This mirrors covidcast behavior.

No geo_type = "county" default please. The wait [to be able to do something again after realizing one's mistake of not specifying another geo_type desired] might be shorter with epidatr but I expect there's still a wait. (I'd also vote for no geo_type default at all. geo_values = "*" default maybe...)

[Another argument against geo_type = "county" default: this could cause mixups between megacounties and state-level data.]

@nmdefries
Copy link
Contributor Author

Remaining tasks moved to their own issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants