add setnafill support for character vectors#7748
Conversation
MichaelChirico
left a comment
MichaelChirico
left a comment
There was a problem hiding this comment.
I guess we should add a test of set() behavior per se where we ensure the delta of addresses is as intended?
| } | ||
| } | ||
| SEXP ans = R_NilValue; | ||
| if (!binplace) { |
There was a problem hiding this comment.
nit: we could also switch the order of branches now to be the more natural if (binplace) { ... } else { ... }
(we could also consider defining bool copy = !LOGICAL(inplace)[0]; since I find the b prefix a bit confusing)
Generated via commit fa2c86d Download link for the artifact containing the test results: ↓ atime-results.zip
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7748 +/- ##
==========================================
- Coverage 99.04% 99.04% -0.01%
==========================================
Files 87 87
Lines 17065 17064 -1
==========================================
- Hits 16902 16901 -1
Misses 163 163 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| for (R_len_t i=0; i<nx; i++) { | ||
| vans[i] = ((ans_t) { .dbl_v=dx[i], .int_v=ix[i], .int64_v=i64x[i], .status=0, .message={"\0","\0","\0","\0"} }); | ||
| SEXP xi = VECTOR_ELT(x, i); | ||
| vans[i] = ((ans_t) { .dbl_v=dx[i], .int_v=ix[i], .int64_v=i64x[i], .char_v=isString(xi) ? xi : R_NilValue, .status=0, .message={"\0","\0","\0","\0"} }); |
There was a problem hiding this comment.
Isn't it the first time we use .char_v field of ans_t?
Line 13 in 188dd7b
says to not use it in parallel
3 participants
Closes #7586 #3992
Maybe I'm overlooking smth here because the addition seems to easy