Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

setDT not always sets ".internal.selfref" #6864

Open
rikivillalba opened this issue Mar 12, 2025 · 3 comments
Open

setDT not always sets ".internal.selfref" #6864

rikivillalba opened this issue Mar 12, 2025 · 3 comments

Comments

@rikivillalba
Copy link
Contributor

rikivillalba commented Mar 12, 2025

Reading #6862 I found this behaviour:

ds = copy(datasets::iris)
x = setDT(get("ds"))
attr(ds, ".internal.selfref")   # <pointer...>

ds = copy(datasets::iris)
x = setDT(get0("ds"))            # note get0
attr(ds, ".internal.selfref")    # NULL

Both ds's set as data.table's but in the second form, ds lacks selfref.
AFAIK setalloccol() is shallow copying ds, but then setDT fails to reassign it to the symbol ds because not recognizing 'get0' bindimg.
Arguably there is no easy way to detect the binding for every possible call that returns a data.table bound to a symbol, so I think setDT should warn when it is unable to assign it back -if a shallow copy is made-. or if ds lacks selfref before returning.
However, x does have it, and thus identical(ds, x) fails in second case.
Related perhaps #6754

@MichaelChirico
Copy link
Member

MichaelChirico commented Mar 12, 2025

I think in this case we can continue the kludge pretty easily -- there's already logic for get(), it was just written before get0() existed (? or at least was more well-known):

} else if (name %iscall% "get") { # #6725

Note also #6702

@venom1204
Copy link
Contributor

Hi @MichaelChirico @rikivillalba

Can I work on this issue .
As suggested by Michael, I'll update the existing logic in setDT() to handle get0() similarly to get():

else if (name %iscall% "get" || name %iscall% "get0")

Thanks!

@MichaelChirico
Copy link
Member

Let's get #6802 finished first, since it is likely to generate conflict with that change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants