You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The idea of the find_start_date() and find_end_date() functions is excellent, and I find that the latter works exactly as intended. A really nice and useful function. However, I think there is an issue with find_start_date().
When trying to find the appropriate start date, you want the later of recall_begin, date_join, and date_born, not the earlier. As it is currently implemented, it always returns the beginning of the recall period, even if someone joins or is born during the recall period. It works as intended for end dates, though, as you want the earlier date, not the later.
The text was updated successfully, but these errors were encountered:
hey jonny - thanks for this. See what you are saying but its hard to code value judgements... e.g. if both date_join and date_born exist then which do you choose? Really that is a data quality checks question that should happen before passing vars to the function.
I think the best solution is to leave as is, i.e. returning the earliest of the variables given. However you do not to pass recall_begin to the function, this way if neither of the others exist it returns NA and then you can replace NA with recall_begin with a simple ifelse().
I guess we could make recall_begin a separate param and then hardcode the above...
The idea of the
find_start_date()
andfind_end_date()
functions is excellent, and I find that the latter works exactly as intended. A really nice and useful function. However, I think there is an issue withfind_start_date()
.When trying to find the appropriate start date, you want the later of
recall_begin
,date_join
, anddate_born
, not the earlier. As it is currently implemented, it always returns the beginning of the recall period, even if someone joins or is born during the recall period. It works as intended for end dates, though, as you want the earlier date, not the later.The text was updated successfully, but these errors were encountered: