tables() supports recursive search for nested data.tables via recursive=TRUE argument #7141

venom1204 · 2025-07-08T16:52:10Z

This PR adds a new recursive=TRUE argument to the tables() function to detect data.table objects nested within plain lists,

Changes introduced:

Added recursive=FALSE (default) to preserve backward compatibility.

When recursive=TRUE, tables():

Recursively traverses plain lists in the environment to find nested data.tables.
Skips data.frame and data.table containers to avoid descending into list-columns.
Handles cyclic list references safely using an internal visited environment.
Generates user-friendly names for nested objects (e.g., mylist$dt, nested[[1]][[2]]).

The recursive logic is implemented iteratively for performance and stack safety.

hi @MichaelChirico can you please have a look when you have time.
thanks..

codecov · 2025-07-08T16:59:17Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (master@c27ec26). Learn more about missing BASE report.

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #7141   +/-   ##
=========================================
  Coverage          ?   98.79%           
=========================================
  Files             ?       81           
  Lines             ?    15286           
  Branches          ?        0           
=========================================
  Hits              ?    15102           
  Misses            ?      184           
  Partials          ?        0

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2025-07-08T17:09:47Z

HEAD=issue_2606 slower P<0.001 for memrecycle regression fixed in #5463
HEAD=issue_2606 slower P<0.001 for DT[by,verbose=TRUE] improved in #6296

Generated via commit dec9834

Download link for the artifact containing the test results: ↓ atime-results.zip

Task	Duration
R setup and installing dependencies	2 minutes and 59 seconds
Installing different package versions	1 minutes and 19 seconds
Running and plotting the test cases	2 minutes and 34 seconds

venom1204 · 2025-07-11T07:02:08Z

hi @MichaelChirico ,can you please review it if has met the required conditions.

working of the fix -

Activation: The new search logic only runs if recursive is TRUE. Otherwise, the function behaves exactly as it did before.
The "To-Do List": It creates a to-do list (an agenda) that starts with all the objects in your current workspace.

The Work Loop: The code then loops through this agenda one item at a time until it's empty. Inside the loop:

If it's a data.table: It's a match! The item is saved to a final results list.
If it's a list: This is where it goes deeper. All of the list's contents are added to the front of the to-do list, each with a descriptive name (like my_list$item).
If it's anything else: It's simply ignored.
The Safety Net (Cycle Detection): This is the most important part. Before exploring a list, the code checks a high-speed "memory" to see if it has visited that exact list before. If so, it skips it. This prevents the code from ever getting stuck in an infinite loop.
Displaying Results: After the search is complete, the function takes all the data.tables it found and formats them into the same neat summary table as before.

MichaelChirico · 2025-07-11T16:44:42Z

I will take some time to review as this PR is non-trivial, thanks for your patience.

MichaelChirico · 2025-07-11T23:30:05Z

inst/tests/tests.Rraw

@@ -21365,3 +21365,28 @@ test(2328.1, levels(droplevels(DT)$f), character())
 DT[, i := integer()]
 DT[, f2 := factor()]
 test(2328.2, droplevels(DT), data.table(f=factor(), i=integer(), f2=factor()))
+
+#2606
+test(2329.1, {


please avoid using many expressions inside test(); also, note that the default for y is TRUE, so omit it if that's the target value you're testing.

For tables(), it's a bit troublesome to test because it's specific to the current environment. I recommend trying something like this for these tests:

local({ dt1 <- data.table(a=1) tables() })

That is assured to only pick up objects defined within the local({...}) scope.

I updated it 👍

MichaelChirico · 2025-07-11T23:31:49Z

R/tables.R

+      found_items = lapply(w, function(i) list(name=names[i], obj=obj[[i]]))
+    }
+  }
+  if (!length(found_items)) { # MODIFIED: Check `found_items` instead of `w`


we don't need this comment

…o issue_2606

…into issue_2606

MichaelChirico · 2025-07-12T21:26:50Z

inst/tests/tests.Rraw

@@ -21426,3 +21426,27 @@ test(2330.7, as.data.table(list(z), keep.rownames=TRUE), data.table(rn=rep("", 3

 M <- matrix(1:6, nrow=3, dimnames=list(rep("", 3), c("V1", "V2")))   #  test of list(M) for empty-rowname'd matrix input
 test(2330.8, as.data.table(list(M), keep.rownames=TRUE), data.table(rn=rep("", 3), V1=1:3, V2=4:6))
+
+#2606 Recursive tables() naming convention
+test(2331.1, local({


I would combine these three into one test:

local({ lst_named = list(...) lst_unnamed = list(...) nested = list(...) test(, tables(recursive=TRUE)$NAME, c(...)) })

Please also include tests that cover:

Mixed named/unnamed simple lists: list(data.table(..), b=data.table(...))

Mixing named/unnamed in a nested setting too

MichaelChirico · 2025-07-12T21:29:33Z

inst/tests/tests.Rraw

+  "nested$l1$l2" %in% tables(recursive = TRUE)$NAME
+}))
+test(2331.4, local({
+  cycle <- list()


for cycles, I don't think list() is the case we should care about, AFAIK it's not possible to create an address cycle in a list() because of R's copy-on-write semantics. Rather, we should be testing the behavior of an environment with such a cycle, c.f.

l = list(1) l[[2]] = l address(l) == address(l[[2]]) # [1] FALSE e =list2env(list(a = 1)) e$cpy = e address(e) == address(e$cpy) # [1] TRUE

…o issue_2606

MichaelChirico · 2025-07-14T21:13:28Z

inst/tests/tests.Rraw

@@ -21489,3 +21489,45 @@ if (test_bit64) {
 # support rotate idiom
 M1 = M2 = matrix(1:4, nrow=2)
 test(2332.81, {M1[]=frev(M1); M1}, {M2[]=rev(M2); M2})
+
+#2606 Recursive tables() naming convention
+test(2333.1, local({


please read my response more carefully, this is not what I recommended

I’ve merged the three tests into a single test case as suggested. Could you please let me know if there’s anything that might still be missing ? —thanks again!

…o issue_2606

MichaelChirico · 2025-07-15T16:10:03Z

inst/tests/tests.Rraw

@@ -21523,3 +21523,40 @@ test(2332.81, {M1[]=frev(M1); M1}, {M2[]=rev(M2); M2})

 # regression test of edge case report #4964
 test(2333, as.expression(data.table(a = 1))[["a"]], 1)
+
+#2606 Recursive tables() naming convention
+test(2334.1, local({


again: don't use local() inside test(), but the other way around.

i have submitted the modified test do they align with the requirements .
thankks

MichaelChirico · 2025-07-15T17:50:04Z

R/tables.R

+  } else {
+    w = which(vapply_1b(objs, is.data.table))
+    if (length(w)) {
+      found_items = lapply(w, function(i) list(name=names[i], obj=obj[[i]]))


does this lead to new copies of the data.table? please check the performance, e.g. consider adding an {atime} test here that confirms tables() performs the same under !recursive before and after this change.

No copies are made. found_items stores a list of names and references (pointers) to the original data.tables.

…into issue_2606

tdhock · 2025-07-21T14:26:41Z

.ci/atime/tests.R

@@ -286,5 +286,18 @@ test.list <- atime::atime_test_list(
    Slow = "548410d23dd74b625e8ea9aeb1a5d2e9dddd2927",   # Parent of the first commit in the PR (https://github.com/Rdatatable/data.table/commit/548410d23dd74b625e8ea9aeb1a5d2e9dddd2927)
    Fast = "c0b32a60466bed0e63420ec105bc75c34590865e"),  # Commit in the PR (https://github.com/Rdatatable/data.table/pull/7144/commits) that uses a much faster implementation

+  "tables() !recursive refactor in #2606" = atime::atime_test(
+    N = as.integer(10^seq(1, 4, by=0.5)),


you should omit N unless you need it to be different from the global definition above (and I don't think you do in this case).

N = as.integer(10^seq(1, 7, by=0.25)),

tdhock · 2025-07-21T14:27:50Z

.ci/atime/tests.R

+    "before" = "5bb645082aa5c4a295cdd211a5a75c849d590b75",
+    "after" = "8978cf201d8d228506e1e96d3eda7e542471720a"),


please remove double quotes from "before" and "after" (they are only necessary for non-syntactic names)

also please add comments to explain where these version numbers come from.

tdhock · 2025-07-21T14:29:15Z

atime job errors with this output

* DONE (data.table.8978cf201d8d228506e1e96d3eda7e542471720a)
Registered S3 methods overwritten by 'data.table.5bb645082aa5c4a295cdd211a5a75c849d590b75':
  method                   from                                               
  all.equal.data.table     data.table.c0b32a60466bed0e63420ec105bc75c34590865e
...
  sort_by.data.table       data.table.c0b32a60466bed0e63420ec105bc75c34590865e
Error: Error in `[.data.frame`(x, i) : undefined columns selected
Calls: <Anonymous> ... <Anonymous> -> system.time -> do.call -> <Anonymous>
Timing stopped at: 0.06 0.002 0.062
Execution halted
Error: Process completed with exit code 1.

this suggests there is some issue with the version of data.table it is trying to run, 5bb6450, which is the "before" historical version introduced in the new atime test.

…o issue_2606

…into issue_2606

tdhock · 2025-07-21T17:06:36Z

.ci/atime/tests.R

+    before = "2b191aec3df51675a7ab5e6701384a8b89470af6",  # The merge-base parent commit for PR #2606
+    after =  "66024e6ea5304ae059b18c71de9973715ddb77ce"),   # The final commit of PR #2606


please use the same template for comments as the other test cases (inlcude URLs)
also "final commit" does not make sense for a PR which has not yet been merged

tdhock · 2025-07-21T17:24:29Z

.ci/atime/tests.R

@@ -300,8 +300,8 @@ test.list <- atime::atime_test_list(
      data.table::tables(env = test_env, silent = TRUE, index = TRUE)
      NULL
    },
-    before = "2b191aec3df51675a7ab5e6701384a8b89470af6",  # The merge-base parent commit for PR #2606
-    after =  "66024e6ea5304ae059b18c71de9973715ddb77ce"),   # The final commit of PR #2606
+    before = "2b191aec3df51675a7ab5e6701384a8b89470af6",  # Merge-base parent commit for PR #2606 (https://github.com/Rdatatable/data.table/commit/2b191aec3df51675a7ab5e6701384a8b89470af6


please read https://github.com/Rdatatable/data.table/wiki/Performance-testing#documentation-of-historical-commits
the link should be to a PR page (on a page where we can see how the commit id is related to the PR), not to the commit page (on which we do not see anything related to PR2606).

also there is no PR2606, but there is an issue 2606.

also there is no PR2606, but there is an issue 2606.
that was a type error it was meant to be 7141

tdhock · 2025-07-21T20:01:57Z

.ci/atime/tests.R

-    before = "2b191aec3df51675a7ab5e6701384a8b89470af6",  # Merge-base parent commit for PR #2606 (https://github.com/Rdatatable/data.table/commit/2b191aec3df51675a7ab5e6701384a8b89470af6
-    after  = "66024e6ea5304ae059b18c71de9973715ddb77ce"),   # Commit in the PR (https://github.com/Rdatatable/data.table/pull/7141/commits) 
+    before = "2b191aec3df51675a7ab5e6701384a8b89470af6", # Parent of the first commit in the PR (https://github.com/Rdatatable/data.table/commit/2b191aec3df51675a7ab5e6701384a8b89470af6)
+    after =  "66024e6ea5304ae059b1e96d3eda7e542471720a"),  # Commit in the PR (https://github.com/Rdatatable/data.table/pull/7141/commits) that uses a much faster implementation


If this is much faster then please change before to Slow and after to Fast

It was only for checking, to ensure there’s no performance regression after adding the recursive argument.

…o issue_2606

venom1204 added 2 commits July 8, 2025 16:48

added condition

6fee38c

news entry

9a89e90

venom1204 requested a review from MichaelChirico as a code owner July 8, 2025 16:52

Update tables.R

0e39296

MichaelChirico reviewed Jul 11, 2025

View reviewed changes

venom1204 added 5 commits July 12, 2025 02:31

Merge branch 'master' of https://github.com/Rdatatable/data.table int…

a04d3ed

…o issue_2606

updates tests and comments

f1cc4c8

Merge branch 'issue_2606' of https://github.com/Rdatatable/data.table …

0c39e3b

…into issue_2606

..

088bb3e

updated tes

1ae5f7d

MichaelChirico reviewed Jul 12, 2025

View reviewed changes

venom1204 and others added 8 commits July 13, 2025 04:03

updaed testst

bb830f8

..

2a54898

,,,

4f5ee7f

added test for uncovered line

04cf5ff

..

405d693

Merge branch 'master' of https://github.com/Rdatatable/data.table int…

26bd48a

…o issue_2606

..

597a5b3

Merge branch 'master' into issue_2606

dccca8f

MichaelChirico reviewed Jul 14, 2025

View reviewed changes

venom1204 added 3 commits July 15, 2025 08:18

updated tests

b69930e

Merge branch 'master' of https://github.com/Rdatatable/data.table int…

1b02090

…o issue_2606

..

acdb19d

MichaelChirico reviewed Jul 15, 2025

View reviewed changes

venom1204 added 2 commits July 16, 2025 09:29

Merge branch 'master' into issue_2606

dec9834

added atime test

c757efa

venom1204 requested review from tdhock and Anirban166 as code owners July 16, 2025 09:25

venom1204 added 7 commits July 16, 2025 09:42

table()

1ab6068

..

e335ccd

..

affa183

tests.R

e6aebc8

final

9746a05

..

9018240

Merge branch 'issue_2606' of https://github.com/Rdatatable/data.table …

a4c7e7a

…into issue_2606

venom1204 mentioned this pull request Jul 19, 2025

main discussion page for gsoc-2025 (VINIT) venom1204/discussion#1

Open

tdhock requested changes Jul 21, 2025

View reviewed changes

venom1204 added 4 commits July 21, 2025 20:18

Merge branch 'master' into issue_2606

f6ee96d

Merge branch 'master' of https://github.com/Rdatatable/data.table int…

ca6637c

…o issue_2606

Merge branch 'issue_2606' of https://github.com/Rdatatable/data.table …

66024e6

…into issue_2606

updated format

16cec98

venom1204 requested a review from tdhock July 21, 2025 17:03

tdhock reviewed Jul 21, 2025

View reviewed changes

venom1204 requested a review from tdhock July 21, 2025 17:21

tdhock reviewed Jul 21, 2025

View reviewed changes

Merge branch 'master' of https://github.com/Rdatatable/data.table int…

5291335

…o issue_2606

venom1204 force-pushed the issue_2606 branch from ea3c831 to 5291335 Compare July 22, 2025 00:08

venom1204 added 4 commits August 18, 2025 11:55

Merge branch 'master' of https://github.com/Rdatatable/data.table int…

c0e3db7

…o issue_2606

merged

074a46a

atime test update

e63b553

..

d6aa6b5

		"before" = "5bb645082aa5c4a295cdd211a5a75c849d590b75",
		"after" = "8978cf201d8d228506e1e96d3eda7e542471720a"),

		before = "2b191aec3df51675a7ab5e6701384a8b89470af6", # The merge-base parent commit for PR #2606
		after = "66024e6ea5304ae059b18c71de9973715ddb77ce"), # The final commit of PR #2606

tables() supports recursive search for nested data.tables via recursive=TRUE argument #7141

Are you sure you want to change the base?

tables() supports recursive search for nested data.tables via recursive=TRUE argument #7141

Uh oh!

Conversation

venom1204 commented Jul 8, 2025

Uh oh!

codecov bot commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

venom1204 commented Jul 11, 2025

Uh oh!

MichaelChirico commented Jul 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tdhock commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Jul 8, 2025 •

edited

Loading

github-actions bot commented Jul 8, 2025 •

edited

Loading

tdhock commented Jul 21, 2025 •

edited

Loading