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 list-column dims #4154

Merged
merged 6 commits into from
Jan 3, 2020
Merged

Print list-column dims #4154

merged 6 commits into from
Jan 3, 2020

Conversation

TysonStanley
Copy link
Member

Closes #3671

Prints dims with list-columns. For example:

DT = data.table(
  x = 1:2,
  y = list(data.table(x = 1,
                      y = 1),
           data.table(x = 2,
                      y = 2))
)
print(DT, class=TRUE)
#>        x                 y
#>    <int>            <list>
#> 1:     1 <data.table[1x2]>
#> 2:     2 <data.table[1x2]>

DT = data.table(
  x = 1:2,
  y = list(list(x = 1,
                y = c("yes", "no")),
           list(x = 2,
                y = 2))
)
print(DT, class=TRUE)
#>        x         y
#>    <int>    <list>
#> 1:     1 <list[2]>
#> 2:     2 <list[2]>

This provides some additional information without printing too much and costs relatively little since it only applies to what is being printed, not the entire data table.

@codecov
Copy link

codecov bot commented Dec 31, 2019

Codecov Report

Merging #4154 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4154      +/-   ##
==========================================
+ Coverage   99.53%   99.54%   +<.01%     
==========================================
  Files          72       72              
  Lines       13911    13915       +4     
==========================================
+ Hits        13847    13851       +4     
  Misses         64       64
Impacted Files Coverage Δ
R/print.data.table.R 100% <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 3d24927...f6d6960. Read the comment docs.

Copy link
Member

@jangorecki jangorecki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for PR! looks good but we need more tests. Please add tests for: S4 class object, named vector, matrix, arrays (1D, 3D and 4D). 1D array - a vector having a dimension of length 1: array(1:4, c(2,2))[1,, drop=FALSE].

@TysonStanley
Copy link
Member Author

If I understand correctly, the current way for data.table to print named vectors and arrays is to present the first 6 elements and then ... so we wouldn't be printing the dimensions for those. As it is, the print method uses is.atomic() to make the decision about the printing of the class name (and with this PR the dimensions). Would we want to change that and have the class and dimensions printed for named vectors and arrays?

As for the s4 classes, I've added functionality to handle those and have tests for it.

@TysonStanley
Copy link
Member Author

TysonStanley commented Jan 2, 2020

To clarify:

s4class <- setClass("ex_class", slots = list(x = "integer", y = "character", z = "numeric"))
array1 = array(1:4, 2)
array2 = array(1:8, c(2,2))
array3 = array(1:16, c(2,2,2))

DT = data.table::data.table(
  int = 1L:2L,
  list = list(list(x = 1,
                y = c("yes", "no")),
           list(x = 2,
                y = 2)),
  s4 = list(s4class(x = 1L, y = "thing1", z = 3.4),
            s4class(x = 2L, y = "thing2", z = 2.5)),
  array1 = list(array1[1, drop = FALSE],
                array1[2, drop = FALSE]),
  array2 = list(array2[,1, drop = FALSE],
                array2[,2, drop = FALSE]),
  array3 = list(array3[,,1, drop = FALSE],
                array3[,,2, drop = FALSE]),
  named_vec = list(c("var1" = 1, "var2" = 2),
                   c("var1" = 3, "var2" = 4))
  )
print(DT, class=TRUE)
#>      int      list            s4 array1 array2  array3 named_vec
#>    <int>    <list>        <list> <list> <list>  <list>    <list>
#> 1:     1 <list[2]> <ex_class[3]>      1    1,2 1,2,3,4       1,2
#> 2:     2 <list[2]> <ex_class[3]>      2    3,4 5,6,7,8       3,4

So the arrays and named vectors don't print the class as of right now.

@MichaelChirico
Copy link
Member

I'm happy if codecov is happy re:tests :)

Looks good overall; I just wonder whether we should patch up #3414 first and add this on to that

@TysonStanley
Copy link
Member Author

That makes sense to me. Seems like #3414 is definitely broader in scope.

Question: Does #3414 mostly have to do with printing list columns with custom classes and arrays/named vectors/matrices? That’s what it seemed to me from the discussion about using toString(). Or does it also include individuals being able to customize printing of, say, data tables in a list column?

@MichaelChirico
Copy link
Member

Technically that's enabled, yep, through the format_list_item generic. So users could define format_list_item.data.table.

@TysonStanley
Copy link
Member Author

I like the flexibility. As for this PR, I’ll wait to hear back from you once things are more clear on #3414

@mattdowle mattdowle added this to the 1.12.9 milestone Jan 3, 2020
@mattdowle
Copy link
Member

Btw, why is your id @TysonStanley but your name Tyson Barrett? It doesn't matter really other than it catches me out w.r.t. to checking your name is in DESCRIPTION, and when thanking you in news.

@mattdowle mattdowle merged commit d9b7ad5 into master Jan 3, 2020
@mattdowle mattdowle deleted the print-list-dims branch January 3, 2020 02:09
@TysonStanley
Copy link
Member Author

Great question. My middle name is Stanley and I thought I was being creative but, in hindsight, would have gone with something like "@tysonbarrett" instead... Thanks for working around it.

@jangorecki
Copy link
Member

I really like the idea that Michael said about using format_list_item to achieve what we need in this PR. Do you think you could follow-up later on with required changes when we have generic method in place?

@mattdowle
Copy link
Member

@jangorecki anyone can follow up. Tyson has achieved what he wanted, which closes an issue. We can't ask for more than that.

@jangorecki jangorecki modified the milestones: 1.12.11, 1.12.9 May 26, 2020
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.

show dimensions of list columns with DT
4 participants