force class reset in as.data.table.data.frame

[ben-schwen]

Closes #6874

• data.table(s4)
• as.data.table(no.reset)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.59%. Comparing base (0909048) to head (0e53a5a).
Report is 26 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #6875   +/-   ##
=======================================
  Coverage   98.59%   98.59%           
=======================================
  Files          79       79           
  Lines       14661    14665    +4     
=======================================
+ Hits        14455    14459    +4     
  Misses        206      206           

☔ 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]

Generated via commit 0e53a5a

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

Task	Duration
R setup and installing dependencies	4 minutes and 45 seconds
Installing different package versions	8 minutes and 53 seconds
Running and plotting the test cases	2 minutes and 41 seconds

[ben-schwen]

Not sure how to best mention the regression of #5427 in NEWS 😭

[MichaelChirico]

maybe we should just directly call as.data.table.data.frame() after we reun as.data.frame(), just like we run as.data.table.data.table() in the line before, WDYT?

[ben-schwen]

maybe we should just directly call as.data.table.data.frame() after we reun as.data.frame(), just like we run as.data.table.data.table() in the line before, WDYT?

Good point. We save a dispatch this way :)

[MichaelChirico]

What I had in mind is even simpler:

Suggested change

	if (!identical(class(x), "data.frame")) {
	class_orig = class(x)
	x = as.data.frame(x)
	if (identical(class(x), class_orig)) setattr(x, "class", "data.frame") # cater for cases when as.data.frame can generate a loop #6874
	return(as.data.table.data.frame(x, keep.rownames=keep.rownames, key=key, ...))
	}
	if (!identical(class(x), "data.frame")) return(as.data.table.data.frame(as.data.frame(x), keep.rownames=keep.rownames, key=key, ...))

[ben-schwen]

but this would put us back to the infinity recursion?

[MichaelChirico]

true true. for my suggestion we'd have to separate the rest of the body into an implementation function and call that to skip the input check at the top. not sure it's that beneficial actually.

[MichaelChirico]

Please help to cherry-pick this into the patch branch