Wqp fixes #58
Wqp fixes #58
Conversation
…s only min/mean/max statcodes and uses them appropriately, drops observations where activity start date does not equal end date (except where there is only one obs per site-date) or resolves date issues when there are collection dates in comments. Also does not calculate min and max when there is only a single observation.
|
@lekoenig, FYI |
Sure, I can definitely take a look. Does a review early next week work for you, @limnoliver? |
Yep, sure does. Thanks Lauren. |
There was a problem hiding this comment.
This looks good to me, @limnoliver! I reviewed the code but did not run it myself or inspect the output since I do not have access to the shared data cache for this repo. I've added a few questions - mostly for my own reference. I made one suggestion but it is very minor and shouldn't change the output so I've gone ahead and approved this PR.
For reference, do you mind adding somewhere in this PR what sort of numbers you're seeing as a result of these code changes (percent or number of records dropped at the national scale)?
| temperature_min_daily = min(ResultMeasureValue), | ||
| temperature_max_daily = max(ResultMeasureValue), | ||
| n_obs = n(), | ||
| dat_reduced_statcode <- ungroup(dat_reduced) %>% |
There was a problem hiding this comment.
What is the grouping structure for dat_reduced? You could consider moving the ungroup() step to line 95 to avoid repeating that step in line 96 and then again in line 109.
5_data_munge/src/munge_wqp_files.R
Outdated
|
|
||
| resolve_statcodes <- function(in_ind, out_ind) { | ||
| dat <- readRDS(sc_retrieve(in_ind, remake_file = 'getters.yml')) %>% | ||
| ungroup() %>% |
There was a problem hiding this comment.
Similar to my comment above, I wonder about ungrouping the data frame earlier on in the pipeline. Is it expected that '5_data_munge/out/wqp_data_streams.rds.ind' has some grouping structure that should be maintained?
This is a minor formatting suggestion and I don't expect it to change the output of the pipeline, so feel free to take or leave this suggestion.
| message(paste(nrow_o - nrow(dat), 'observations were dropped due to estimation, blank correction, or statcode that was not mean, min, max')) | ||
| # for some data, the start and end dates are different, and data providers | ||
| # seem to be using these as a date range of the whole dataset | ||
| # sometimes, the proper collection date is in the comment field |
There was a problem hiding this comment.
sometimes, the proper collection date is in the comment field
😢
| `ActivityStartTime/Time` = newActivityStartTime) | ||
|
|
||
| # print message about date recoveries | ||
| message(paste(nrow(range_dates), 'observations with mismatching start/end dates were recovered by extracting collection dates from comments')) |
There was a problem hiding this comment.
Nice, way to recover this info!
There was a problem hiding this comment.
For posterity: 2115448 observations with mismatching start/end dates were recovered by extracting collection dates from comments
There was a problem hiding this comment.
Wow, that number is higher than I was expecting!
…have to read in big file twice. Not sure why indicators rebuilt, but confident there weren't downstream chances because I did an scmake on the final traget in 5_munge and it did not rebuild.
Some WQP fixes that the trends group pointed out to us. It handles the fields
StatisticalBaseCodeas well as some date parsing issues, where cooperators were reporting means across several dates, or dates were reported as the start of the project rather than the date of collection. This PR rebuilds all data downstream of these changes.