Repository navigation
:= could warn/error when RHS is ambiguous #3077
Description
Activity
(Looking at the title...) Seems like a useful precaution in any
jcall, not just:=...?Thinking again, maybe this can be solved up front by adding gateways in
setDT,as.data.table.*,fread... ifcheck.namesis `FALSE, check the names anyway and warn by default?Not sure it makes sense to run this code repeatedly in every
jcallYeah, a warning upstream like you suggest would be more useful to me, though there are so many points at which data.tables are created, it may be too much of a headache that way, too. (Eg, besides the ones you mention, there's even
rbindlist(list(list(x = 1, x = 2, x = 3))))For me,
:=isn't special relative to other DT[...] contexts where I might make such a mistake:SC[, table(Nomination)];SC[, .N, by=Nomination]. I don't feel strongly about it, though and this problem hardly affects me (since I work with predictable data structures these days).- right, since it's such an edge case and fread/setDT already probably cover 95% of data.table creation (fundamental flaw of input data really), I'd be fine letting users sort it out themselves in other cases... the behavior's unexpected only to the extent that you don't understand the table in the first place, which is relatively more likely at the fread/setDT junctures as well…On Thu, Sep 27, 2018, 12:37 PM Frank ***@***.***> wrote: Yeah, a warning upstream like you suggest would be more useful to me, though there are so many points at which data.tables are created, it may be too much of a headache that way, too. (Eg, besides the ones you mention, there's even rbindlist(list(list(x = 1, x = 2, x = 3)))) For me, := isn't special relative to other DT[...] contexts where I might make such a mistake: SC[, table(Nomination)]; SC[, .N, by=Nomination]. I don't feel strongly about it, though and this problem hardly affects me (since I work with predictable data structures these days). — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3077 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHQQdWrwG0vkj4xE6DNSKA73j9KTbhRWks5ufFX6gaJpZM4W7u90> .Reacted by Frank
Have a fix for this issue like this:
- allcols = c(names(x), xdotprefix, names(i), idotprefix) - ansvars = setdiff(intersect(av,allcols), bynames) + if (anyDuplicated(used_from_x <- dupintersect(names_x, av))) { + dupcols = unique(used_from_x[duplicated(used_from_x)]) + stop("Unable to disambiguate reference to duplicated columns in x: ", brackify(dupcols)) + } + allcols = c(names_x, xdotprefix, names_i, idotprefix) + ansvars = sdvars = setdiff(intersect(allcols, av), bynames)But it breaks tests
1290where there are some rules laid out for howdata.tableis supposed to act with duplicates, namely:######################################## # Extensve testing for "duplicate" names ######################################## # Rules: Basically, if index is directly given in 'j', just those columns are touched/operated on. But if 'column' names are given and there are more than one # occurrence of that column, then it's hard to decide which to keep and which to remove. So, to remove, all are removed, to keep, always the first is kept. # 1) when i,j,by are all absent (or) just 'i' is present then ALL duplicate columns are returned. # 2) When 'with=FALSE' and 'j' is a character and 'notj' is TRUE, all instances of the column to be removed will be removed. # 3) When 'with=FALSE' and 'j' is a character and 'notj' is FALSE, only the first column will be recognised in presence of duplicate columns. # 4) When 'with=FALSE' and 'j' is numeric and 'notj' is TRUE, just those indices will be removed. # 5) When 'with=FALSE' and 'j' is numeric and 'notj' is FALSE, all columns for indices given, if valid, are returned. (FIXES #5688) # 6) When .SD is in 'j', but '.SDcols' is not present, ALL columns are subset'd - FIXES BUG #5008. # 7) When .SD and .SDcols are present and .SDcols is numeric, columns corresponding to the given indices are returned. # 8) When .SD and .SDcols are present and .SDcols is character, duplicate column names will only return the first column, each time. # 9) When .SD and .SDcols are present and .SDcols is numeric, and it's -SDcols, then just those columns are removed. # 10) When .SD and .SDcols are present and .SDcols is character and -SDcols, then all occurrences of that object is removed. # 11) When no .SD and no .SDcols and no with=FALSE, only duplicate column names will return only the first column each time. # 12) With 'get("col")', it's the same as with all character types. # 13) A logical expression in 'j'. # 14) Finally, no tests but.. using 'by' with duplicate columns and aggregating may not return the intended result, as it may operate on column names in some cases.Not sure how married we are to the behavior laid out here (a comment in tests hardly seems like documentation)... thoughts @jangorecki / @mattdowle ?
As for me I would move towards not supporting duplicated column names wherever possible. The only use case I can think about is to produce table for reporting/formatting.
- added a commit that references this issue
on Jul 23, 2020 Hmm with hindsight I'd rather not pursue this. Duplicate names can be a pain but they are a fact of life -- I suspect it would cause more pain than help to impose this on user workflows.
E.g. I've recently dealt with this working with {dbplyr} -- I want to run a query like (distilled):
SELECT A.*, B.* FROM A JOIN B USING (x,y,z)
But
AandBmight have columns besidesx,y,zin common, causing this to fail because {dbplyr} (or maybe tibble/dplyr?) refuse to allow duplicates. The workaround is the much more painful approach of writing out all the column names.Another example: I received
DFand I want to convert to data.table to do some processing. If there are duplicate names in the object I receive and it won't affect me, I'll be annoyed I have to further processDF. If there are duplicates and it will affect me, this change amounts to shuffling the code to deal with duplicates around (from after to beforesetDT()/as.data.table()) -- seems like busywork.
Was scraping the table here:
https://en.wikipedia.org/wiki/List_of_nominations_to_the_Supreme_Court_of_the_United_States
And didn't realize the output had duplicated column names:
nominatedwill attempt to compute the RHS and will select the firstNominationcolumn.This is easily avoided by being careful about
check.namesup front, but if it's easy to check/warn on the fly, we should.