Fix g.impute() NA/NaN error with study_dates_file and check_log() IDate handling#1522
Open
peterolejua wants to merge 1 commit into
Open
Fix g.impute() NA/NaN error with study_dates_file and check_log() IDate handling#1522peterolejua wants to merge 1 commit into
peterolejua wants to merge 1 commit into
Conversation
…c#1521) Guard both study-date trim branches in g.impute() with && \!is.na(...) so a listed start/end date that is not a midnight present in the recording falls into the existing recovery branch instead of producing r4[1:(NA - 1)] -> "NA/NaN argument". Also fix the list2env() copy-paste in the first-midnight recovery branch. Coerce check_log() columns to character so study dates auto-parsed as IDate by data.table::fread() are handled correctly. Add regression tests for out-of-range study dates and check_log IDate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1508 and #1521.
g.impute()
Guards both study-date trim branches with
&& \!is.na(...), so a listed start or end date that is not a midnight present in the recording falls into the existing recovery branch instead of producingr4[1:(NA - 1)] = 1/r4[NA:nrow(r4)] = 1→NA/NaN argument.This happens because
firstmidnighti = midnightsi[grep(firstmidnight, midnights)][1](and the matchinglastmidnightiline) returnsNA_integer_(length 1) whengrep()finds no match, so the existinglength(...) > 0check passes and execution reaches the subscript withNA. Two situations trigger it in practice:This PR also fixes a latent copy-paste in the first-midnight recovery
elsebranch, which restoredlastmidnighti/lastmidnightinstead offirstmidnighti/firstmidnight.check_log()
Adds
log[] = lapply(log, as.character)at the top ofcheck_log(), so study dates thatdata.table::fread()auto-parses asIDateare handled by the text-based date checks. The activity-log path is unaffected (it is already read withcolClasses = "character"). Per @jhmigueles' suggestion in #1508.Verification
NA/NaN argument; patched runs clean.test_StudyDatesFile.R(start/end date outside the recorded dates) and a newtest_check_log.Rfor the IDate coercion. Both pass.