modularize optimization internals #7401
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7401 +/- ##
==========================================
- Coverage 99.02% 99.00% -0.02%
==========================================
Files 87 87
Lines 16806 16893 +87
==========================================
+ Hits 16642 16725 +83
- Misses 164 168 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
No obvious timing issues in HEAD=modular_gforce Generated via commit bb6c6ed Download link for the artifact containing the test results: ↓ atime-results.zip
|
|
I'm also not sure about moving the tests to |
MichaelChirico
left a comment
MichaelChirico
left a comment
There was a problem hiding this comment.
OK, I think last round of review here. Looking great!
Co-authored-by: Michael Chirico <chiricom@google.com>
for the case of `is.null(names(jl__))` this should result in `logical(0)` differently from current version which returns `c(FALSE, FALSE, ...)` Co-authored-by: Michael Chirico <chiricom@google.com>
Co-authored-by: Michael Chirico <chiricom@google.com>
Co-authored-by: Michael Chirico <chiricom@google.com>
R/data.table.R
Outdated
| # Case 2b: list(...) | ||
| else if (this[[1L]] == "list") { | ||
| # also handle c(lapply(.SD, sum), list()) - silly, yes, but can happen | ||
| if (length(this) == 1L) next |
There was a problem hiding this comment.
looks like this case was never actually tested
MichaelChirico
left a comment
MichaelChirico
left a comment
There was a problem hiding this comment.
I think it's ready to go! Bravo! Be sure to revisit all the open review threads and then submit when you're happy :)
|
Great news -- as of now, this PR reduces the cyclomatic complexity of |
4 participants
Mapinstead oflapplyturns GForce off #5336[.data.tableneeds to be more modular. #852Adds arithmetic for GForce as demanded in #3815 but does not add support for blocks in j like
d[, j={x<-x; .(min(x))}, by=y].