setDT generates shallow copy earlier to avoid interfering with attributes of co-bound tables

[MichaelChirico]

Closes #4784. Original author: @OfekShilon.

This is #6470, reborn (again). I tried resolving the merge conflict on that PR but I was rejected trying to push to Ofek's fork:

 ! [remote rejected]   fix4784 -> fix4784 (refusing to allow a GitHub App to create or update workflow `.github/workflows/performance-tests.yml` without `workflows` permission)
error: failed to push some refs to 'https://github.com/OfekShilon/data.table.git'

Just FYI to @OfekShilon -- now that you're a project member, should the need arise, making branches on this repo going forward will avoid such issues.

[github-actions]

Generated via commit 21b33c8

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

Task	Duration
R setup and installing dependencies	3 minutes and 38 seconds
Installing different package versions	7 minutes and 20 seconds
Running and plotting the test cases	2 minutes and 19 seconds

[OfekShilon]

Thank you @MichaelChirico. I feel I should say - not only I don't have access to my original fork, but I'm also currently not working with data.table and do not expect to be contributing in the foreseeable future. If you prefer to exclude me from the project members I completely understand.

[MichaelChirico]

No worries @OfekShilon, you had mentioned similar elsewhere (can't see it right away). My comment is as much of a note to self/fellow readers as to you :)

Right now we don't have any policy for removing ppl from 'Maintainers', so the easier thing is to leave you in there and hope we'll see you back eventually! Thanks for all your contributions over the years 🙏

[MichaelChirico]

@ben-schwen do you think there's any test we should add for your 2nd suggested change (running setalloccol() before setattr() for row.names?

[MichaelChirico]

Answering my own question, run at 899e49b

d1=data.frame(a=1, row.names='b')
d1
#   a
# b 1
d2=d1
setDT(d2)
d1
#   a
# 1 1

[MichaelChirico]

@ben-schwen based on the test failures I think this approach is the right way to go -- rather than relying on setalloccol() doing a shallow copy for us (which IINM it only does in certain cases), we are better of doing it directly. I don't think there's any issue with doing x = .shallow(x) here, right?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.62%. Comparing base (d62aac9) to head (12083a7).
Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #6551   +/-   ##
=======================================
  Coverage   98.62%   98.62%           
=======================================
  Files          79       79           
  Lines       14449    14450    +1     
=======================================
+ Hits        14250    14251    +1     
  Misses        199      199           

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