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

Print zero-column data.frame properly #3363

Merged
merged 3 commits into from Feb 8, 2019

Conversation

heavywatal
Copy link
Contributor

@heavywatal heavywatal commented Feb 6, 2019

The attribute $row.names is preserved in zero-column base::data.frame, while not in data.table. I won't go deeply into this feature difference here, but just want to fix printing function.

attributes(iris[,FALSE])
#> $names
#> character(0)
#> 
#> $class
#> [1] "data.frame"
#> 
#> $row.names
#>   [1]   1   2   3   4   5   6   7   8   9  10  11  12  13  14  15  16  17
#>  [18]  18  19  20  21  22  23  24  25  26  27  28  29  30  31  32  33  34
#>  [35]  35  36  37  38  39  40  41  42  43  44  45  46  47  48  49  50  51
#>  [52]  52  53  54  55  56  57  58  59  60  61  62  63  64  65  66  67  68
#>  [69]  69  70  71  72  73  74  75  76  77  78  79  80  81  82  83  84  85
#>  [86]  86  87  88  89  90  91  92  93  94  95  96  97  98  99 100 101 102
#> [103] 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119
#> [120] 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136
#> [137] 137 138 139 140 141 142 143 144 145 146 147 148 149 150

base::print.data.frame(iris[FALSE,])
#> [1] Sepal.Length Sepal.Width  Petal.Length Petal.Width  Species     
#> <0 rows> (or 0-length row.names)
base::print.data.frame(iris[,FALSE])
#> data frame with 0 columns and 150 rows
base::print.data.frame(iris[FALSE,FALSE])
#> data frame with 0 columns and 0 rows

dtiris = data.table::data.table(iris)
attributes(dtiris[,FALSE]) # row.names are removed
#> $class
#> [1] "data.table" "data.frame"
#> 
#> $row.names
#> integer(0)
#> 
#> $names
#> character(0)
#> 
#> $.internal.selfref
#> <pointer: 0x7fb84001d8e0>

data.table:::print.data.table(dtiris[FALSE,])
#> Empty data.table (0 rows) of 5 cols: Sepal.Length,Sepal.Width,Petal.Length,Petal.Width,Species
data.table:::print.data.table(dtiris[,FALSE]) # OK, 0 rows
#> Null data.table (0 rows and 0 cols)
data.table:::print.data.table(dtiris[FALSE,FALSE])
#> Null data.table (0 rows and 0 cols)

data.table:::print.data.table(iris[,FALSE]) # Error
#> Error in `rownames<-`(`*tmp*`, value = paste0(format(rn, right = TRUE, : attempt to set 'rownames' on an object with no dimensions

Created on 2019-02-06 by the reprex package (v0.2.1)

Copy link
Member

@jangorecki jangorecki left a comment

Before submitting PR it is best to discuss expected behaviour. There is existing issue that covers the case of non-zero rows and zero columns: #2422 please put your comment there.

@heavywatal
Copy link
Contributor Author

@heavywatal heavywatal commented Feb 6, 2019

Sorry I didn't check and mention that discussion. But still I don't think it matters because this PR just fixes the printing functionality for the existing base::data.frame (and tibble). The expected behavior here is to print zero-column data.frame without an error. It improves the consistency with base::print.data.frame without interfering the data.table's policy on zero-column data.

That being said, I would be happy to put my comment on that discussion.

@mattdowle mattdowle added this to the 1.12.2 milestone Feb 8, 2019
@mattdowle
Copy link
Member

@mattdowle mattdowle commented Feb 8, 2019

I added a test and changed the NEWS item to reflect what's really going on here :

Calling `data.table:::print.data.table()` directly (i.e. bypassing method dispatch by
using 3 colons) and passing it a 0-column `data.frame` (not `data.table`) now
works, #3363. Thanks @heavywatal for the PR.

Please note that ::: means you get to the methods directly which are not supposed to be called directly. You're not supposed to use :::. For example, usage of ::: fails R CMD check so won't be allowed on CRAN.

@codecov
Copy link

@codecov codecov bot commented Feb 8, 2019

Codecov Report

No coverage uploaded for pull request base (master@bb9f50f). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #3363   +/-   ##
=========================================
  Coverage          ?   94.74%           
=========================================
  Files             ?       65           
  Lines             ?    12152           
  Branches          ?        0           
=========================================
  Hits              ?    11513           
  Misses            ?      639           
  Partials          ?        0
Impacted Files Coverage Δ
R/print.data.table.R 92.7% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bb9f50f...798e042. Read the comment docs.

@mattdowle mattdowle merged commit 70d09c5 into Rdatatable:master Feb 8, 2019
4 checks passed
@heavywatal heavywatal deleted the print-zero-column branch Feb 18, 2019
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

Successfully merging this pull request may close these issues.

None yet

3 participants