Hi Jesse, thanks for writing. Is there a way to fix your issue in a way that maintains backward compatibility with the existing print output? One of the guiding principles of data.table is stability/back-compatibility https://github.com/Rdatatable/data.table/blob/master/GOVERNANCE.md#the-r-package so it would be much easier to accept a code contribution that does not change what is printed.

Hey Toby, thanks for getting back. The current behavior is actually not backwards compatible - list sub-classes print identically to bare lists through v1.14.10, and it's only in v1.15.0 and later that this "print everything" behavior occurs. The proposal is essentially to revert to the previous behavior, because there are cases where the current behavior is pathological, and I'm not sure the current behavior was actually intended (I can't find anything in NEWS that mentions it).

This formats differently because getS3method("format", class="vctrs_list_of") is defined whereas getS3method("format", class="list") is not. My #6593 does not resolve this.

This may be a frustrating issue. I like the notion that format.data.table honors format.<col-class> if found, but the fact that vctrs::format.vctrs_list_of is simply a wrapper around format.default seems less useful (to me).

If you go down the road of over-riding for this class, then pandora's box of "override this class and that class and that other class" may not be desired, plus what happens if/when vctrs changes its format S3 methods.

While frustrating, I wonder if the better path to resolve this aesthetic request is to request that vctrs::format.vctrs_list_of does something different, in which case it will self-resolve within the data.table ecosystem.

cc @DavisVaughan

I wonder if the better path to resolve this aesthetic request is to request that vctrs::format.vctrs_list_of does something different

I don't think so, no. At the end of the day, we are simply following format.list(), and it seem reasonable that format(<list>) and format(<list-of>) do exactly the same thing.

On the tibble side, both pillar:::pillar_shaft.list() and pillar:::pillar_shaft.vctrs_list_of() exist for controlling how they are printed.

I do not think it is out of the question for you to provide format_col.vctrs_list_of() that gets conditionally registered using an .onLoad hook for the vctrs package. Kind of like this, where in vctrs we specify that "when dplyr is loaded, go ahead and register a vec_restore() method for grouped_df"
https://github.com/r-lib/vctrs/blob/78d9f2b0b24131b5ce2230eb3c2c9f93620b10d9/R/zzz.R#L33-L36

I'm good with including that in my current PR (if that doesn't get too complicated). If yes, I'm assuming dt "should" look like this:

dt
#             list_col       list_of_col
#               <list>      <vctrs_list>
# 1: <data.table[3x1]> <data.table[3x1]>
# 2: <data.table[3x1]> <data.table[3x1]>

I added the header class, should it be vctrs_list_of, vctrs_vctr, or just list?

For data.table users I imagine it might make the most sense if you have the full class, since this probably isn't a super common class to see in the data.table world, and in that case the full explicit name is easier to understand than hiding some class name details.

We have some tooling for showing list<type> where type is an abbreviated name of the class the container is for

Okay. This is starting to become a little more than "add some rider code to a PR", so let me know (@MichaelChirico ) if this is out of scope to be appended (to an already-approved PR).

I think we can do this:

@@ -105,6 +105,14 @@ print.data.table = function(x, topn=getOption("datatable.print.topn"),
       expression = "<expr>", ordered = "<ord>")
     classes = classes1(x)
     abbs = unname(class_abb[classes])
+    abb_na = is.na(abbs)
+    if (any(abb_na) && any(classes[abb_na] %in% c("vctrs_list_of", "vctrs_vctr"))) {
+      if (!requireNamespace("vctrs", quietly=TRUE)) {
+        warningf("Some columns are class 'vctrs_list_of' but package vctrs is not available. Those columns will print as regular objects. Simplify install.packages('vctrs') to obtain the vctrs_list_of format method and print the data again.")
+      } else {
+        abbs[abb_na] <- paste0("<", vapply_1c(x, vctrs::vec_ptype_abbr)[abb_na], ">")
+      }
+    }
     if ( length(idx <- which(is.na(abbs))) ) abbs[idx] = paste0("<", classes[idx], ">")
     toprint = rbind(abbs, toprint)
     rownames(toprint)[1L] = ""
@@ -207,6 +215,10 @@ format_col.default = function(x, ...) {
     format(char.trunc(x), ...) # relevant to #37
 }
 
+format_col.vctrs_list_of = function(x, ...) {
+  vapply_1c(x, format_list_item, ...)
+}
+
 # #2842 -- different columns can have different tzone, so force usage in output
 format_col.POSIXct = function(x, ..., timezone=FALSE) {
   if (timezone) {

and

@@ -199,6 +199,7 @@ export(format_col)
 S3method(format_col, default)
 S3method(format_col, POSIXct)
 S3method(format_col, expression)
+S3method(format_col, vctrs_list_of)
 export(format_list_item)
 S3method(format_list_item, default)
 S3method(format_list_item, data.frame)

Which would result in

dt
#             list_col       list_of_col
#               <list>    <list<dt[,1]>>
# 1: <data.table[3x1]> <data.table[3x1]>
# 2: <data.table[3x1]> <data.table[3x1]>

@MichaelChirico thoughts on that?

That will induce a Suggests dependency on {vctrs} so it'll be a "no".

Turns out there is a trivial fix for the bug at hand: #6637.