-
Notifications
You must be signed in to change notification settings - Fork 997
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
Various fixes for R 3.3.0 #6383
Conversation
Generated via commit 84c7e29 Download link for the artifact containing the test results: ↓ atime-results.zip Time taken to finish the standard R installation steps: 11 minutes and 59 seconds Time taken to run |
R/utils.R
Outdated
# In R 3.3.0, rep_len() strips attributes --> breaks data.table()'s internal recycle() helper. | ||
if (inherits(rep_len(Sys.Date(), 1L), "Date")) { | ||
# NB: safe_rep_len=rep_len throws an R CMD check error because it _appears_ to the AST | ||
# walker that we've used .Internal ourselves (which is not true, but codetools can't tell). |
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.
nice explanation
d[-(1:4), z:=10L] | ||
test(2030.06, .Last.updated, 0L) # inverse empty sub-assign | ||
# On R 3.3.0, .Last.updated gets touched by test() itself and this cascades even to re-assignments | ||
# of .Last.updated like n_updated=.Last.updated, so really force a deep copy by round-tripping from string. |
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.
again thanks for the clear explanation
looks good thanks for the updates. |
GHA R-cmd-check-occasional is also not fully working. I've seen some of these errors in logs there but was unable to debug from the logs alone |
@@ -21,6 +21,15 @@ nan_is_na = function(x) { | |||
stopf("Argument 'nan' must be NA or NaN") | |||
} | |||
|
|||
# In R 3.3.0, rep_len() strips attributes --> breaks data.table()'s internal recycle() helper. | |||
if (inherits(rep_len(Sys.Date(), 1L), "Date")) { |
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.
I just checked and this seems still a problem at 3.6.0
so we might need to hold on to this for quite some time, but it would be best to check at which version this is fixed in R
and add a comment
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 for checking. I suspected that might be the case -- I checked the R NEWS and NEWS.3 files and didn't see anything relevant here (searching 'attrib', 'rep_len'), and glanced at the rep_len()
blame to see if anything about attributes popped out, no luck.
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.
Actually looking again, I think it's this:
Then rep_len()
on Date
dispatches to rep.Date()
.
That puts us in R 4.0.0:
Besides the test changes for
.Last.updated
, AFAICT all of these changes are dev-only, hence no NEWS item.