Skip to content

key(as.data.table(x, key = <key>)) is inconsistent, varies based on x #6859

Description

@brookslogan
  • key(as.data.table(x, key = <key>)) is not <key> if x is
    • a tibble
    • a data.table not already keyed by <key>.
  • key(as.data.table(x)) is not NULL if x is a keyed data.table.

# Minimal reproducible example; please be sure to set verbose=TRUE where possible!

suppressPackageStartupMessages({
  library(data.table)
  library(dplyr)
})

# One probably expects key="t" to produce a key of "t":
data.frame(t = c(3:1,4:5), y = 1:5) %>% as.data.table(key = "t") %>% key()
#> [1] "t"
tibble(t = c(3:1,4:5), y = 1:5) %>% as.data.table(key = "t") %>% key()
#> NULL
data.table(t = c(3:1,4:5), y = 1:5) %>% as.data.table(key = "t") %>% key()
#> NULL

# One may also expect omitting `key =` arg to produce unkeyed output; however, 
# that's also not always the case.  The "proper"/expected output is probably
# more up for debate here than above, though.
data.table(t = c(3:1,4:5), key = "t") %>%
  as.data.table() %>%
  key()
#> [1] "t"

# Output of sessionInfo()

sessionInfo()
#> R version 4.4.2 (2024-10-31)
#> Platform: x86_64-pc-linux-gnu
#> Running under: openSUSE Tumbleweed
#> 
#> Matrix products: default
#> BLAS:   /home/fullname/files/sw2/R-4.4.2/lib/libRblas.so 
#> LAPACK: /home/fullname/files/sw2/R-4.4.2/lib/libRlapack.so;  LAPACK version 3.12.0
#> 
#> locale:
#>  [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C              
#>  [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
#>  [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8   
#>  [7] LC_PAPER=en_US.UTF-8       LC_NAME=C                 
#>  [9] LC_ADDRESS=C               LC_TELEPHONE=C            
#> [11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       
#> 
#> time zone: America/Los_Angeles
#> tzcode source: system (glibc)
#> 
#> attached base packages:
#> [1] stats     graphics  grDevices utils     datasets  methods   base     
#> 
#> other attached packages:
#> [1] dplyr_1.1.4        data.table_1.17.99
#> 
#> loaded via a namespace (and not attached):
#>  [1] digest_0.6.37     R6_2.6.1          fastmap_1.2.0     tidyselect_1.2.1 
#>  [5] xfun_0.51         magrittr_2.0.3    glue_1.8.0        tibble_3.2.1     
#>  [9] knitr_1.49        pkgconfig_2.0.3   htmltools_0.5.8.1 generics_0.1.3   
#> [13] rmarkdown_2.29    lifecycle_1.0.4   cli_3.6.4         vctrs_0.6.5      
#> [17] reprex_2.1.1      withr_3.0.2       compiler_4.4.2    tools_4.4.2      
#> [21] pillar_1.10.1     evaluate_1.0.3    yaml_2.3.10       rlang_1.1.5      
#> [25] fs_1.6.5

Created on 2025-03-10 with reprex v2.1.1

# Related issues:

Activity

  1. ben-schwen commented on Mar 10, 2025

    @ben-schwen
    Member

    Thanks for the report. You are right that the results should at least be consistent. I'm slightly in favor adding key arguments to appropriate S3 methods, although I'm a big fan of keys in general.

  2. Mukulyadav2004 commented on Mar 11, 2025

    @Mukulyadav2004
    Contributor

    Hi @ben-schwen ,

    I would like to take up this issue and propose the following changes to improve key handling in as.data.table():
    In as.data.table.data.table:
    Add key parameter that explicitly sets or clears keys using setkeyv() based on the key argument.
    Ensure existing keys are removed when key = NULL.
    In as.data.table.data.frame:
    Set the key after converting from tibble or data.frame, if a key is provided.
    Please let me know if you have any suggestions or if I can proceed with these changes.

  3. ben-schwen commented on Mar 11, 2025

    @ben-schwen
    Member

    Hi @ben-schwen ,

    I would like to take up this issue and propose the following changes to improve key handling in as.data.table(): In as.data.table.data.table: Add key parameter that explicitly sets or clears keys using setkeyv() based on the key argument. Ensure existing keys are removed when key = NULL. In as.data.table.data.frame: Set the key after converting from tibble or data.frame, if a key is provided. Please let me know if you have any suggestions or if I can proceed with these changes.

    Sounds good to me. For the as.data.table.data.frame method make sure that all cased are covered (early return etc.)

  4. jan-glx commented on Mar 24, 2025

    @jan-glx
    Contributor

    this ( 0909048 ) also fixes an issue where as.data.table did not keep.rownames when x is of class() e.g. c(bla, data.frame).

  5. ben-schwen commented on Mar 25, 2025

    @ben-schwen
    Member

    this ( 0909048 ) also fixes an issue where as.data.table did not keep.rownames when x is of class() e.g. c(bla, data.frame).

    True but I don't know if its worth to put as additional test in our suite since

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions