Document how j results are combined with by via rbindlist (issue #7643)#7651
Document how j results are combined with by via rbindlist (issue #7643)#7651mahjabinoyshi wants to merge 4 commits intoRdatatable:masterfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7651 +/- ##
=======================================
Coverage 99.02% 99.02%
=======================================
Files 87 87
Lines 16897 16897
=======================================
Hits 16733 16733
Misses 164 164 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Why not to give explanation in rbindlist a d just refer that by uses rbindlist internally? ?data.table is already so big |
|
@jangorecki Thanks for the feedback! I’m happy to move most of the explanation to ?rbindlist instead. 1)In ?data.table, shorten the new text to something like: Will it be okay? |
|
this text is too long, please shorten. |
|
and please remove test case |
|
@tdhock I’ve updated the PR to shorten the by documentation, kept the new sentence within the by argument section, and removed the test case as requested. |
|
What about keeping addition to ?data.table minimal and just refer to rbindlist section? |
tdhock
left a comment
There was a problem hiding this comment.
please look at Files changed tab before asking for review, to make sure there are no irrelevant diff lines.
This PR updates the
?data.tabledocumentation to clarify how per‑groupjresults are combined whenbyis used.The new paragraph explains that
jis evaluated once per group and the results are bound row‑wise usingrbindlistwithuse.names = FALSE, so columns are matched by position and the first group’s column names are kept. This makes the behavior in cases like #7643 explicit.A small test in
tests/by-rbindlist-use-names-false.Ris given to capture the current behavior so the documentation and implementation stay in sync.Closes #7643.