-
Notifications
You must be signed in to change notification settings - Fork 103
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
Shadrackkibet infillmissingdates #5524
Shadrackkibet infillmissingdates #5524
Conversation
@@ -2562,12 +2564,12 @@ DataSheet$set("public","infill_missing_dates", function(date_name, factors, star | |||
else if(!missing(start_month)) { | |||
if(start_month <= lubridate::month(min_date)) min_date <- as.Date(paste(lubridate::year(min_date), start_month, 1, sep = "-"), format = "%Y-%m-%d") | |||
else min_date <- as.Date(paste(lubridate::year(min_date) - 1, start_month, 1, sep = "-"), format = "%Y-%m-%d") | |||
if(start_month <= lubridate::month(max_date)) max_date <- as.Date(paste(lubridate::year(max_date), 12, 1, sep = "-"), format = "%Y-%m-%d") | |||
else max_date <- as.Date(paste(lubridate::year(max_date) - 1, 12, 1, sep = "-"), format = "%Y-%m-%d") | |||
if(end_month >= lubridate::month(max_date)) as.Date(paste(lubridate::year(max_date), end_month, lubridate::days_in_month(as.Date(paste(lubridate::year(max_date), end_month, 1, sep = "-", format = "%Y-%m-%d"))), sep = "-"), format = "%Y-%m-%d") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shadrackkibet see here that max_date
is not always in month 12. It should be 1 month before start_month
. See addition of end_month
calculation further above.
Then here, the year is either the current year or the next year (depends where the data ends).
Also, the day needs to be at the end of the month, not the start so instead of 1, lubridate::days_in_month
is used to calculate this.
This should now give correct limits when using start_month
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. That is noted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to work fine. I tried with the Ghana data that has short gaps and also doesn't start until February in one station.
I tried after filtering by station and note that it totally ignores a filter. At some stage we should at least note and report when dialogues do this.
That's a good point to be aware of, more general than this dialog. |
@shadrackkibet @rdstern some minor changes to the options display and a correction on the limits with a starting month. Please check this is working ok.