`nrow` should be optional if input is a data frame

[lionel-]

Makes things more complex for the df case:

new_gbif <- function(x) {
  stopifnot(is.data.frame(x))

  tibble::new_tibble(
    out,
    nrow = nrow(out),  # Redundant
    class = "gbif"
  )
}

This behaviour would be consistent with vctrs::new_data_frame().

[hadley]

Are you sure that makes sense? Don't we usually treat the input to constructors as "raw" vectors (i.e. without attributes)?

[lionel-]

I think it makes sense under the vctrs notion of data frame as a base type. @DavisVaughan and I frequently use new_data_frame() on data frames.

[hadley]

Ah yeah, makes sense — makes me wonder if we should be routinely calling vec_data() in the constructor?

[DavisVaughan]

I think I'd like to have nrow always be optional. I think that would be fully consistent with new_data_frame() and more convenient. I sometimes find myself with a list as input that I want to convert to a tibble with new_tibble(). I either have to use a df_size() helper or call new_data_frame() first (since it is always optional there and internally calls the C version of df-size).

df_size <- function (x) {
  if (!is.list(x)) {
    rlang::abort("Cannot get the df size of a non-list.")
  }
  if (length(x) == 0L) {
    return(0L)
  }
  col <- x[[1L]]
  vctrs::vec_size(col)
}

# imagine you get this as input
df_raw <- list(x = 1:5, y = letters[1:5])

# helper approach
tbl <- tibble::new_tibble(df_raw, nrow = df_size(df_raw))

# through new_data_frame first
df <- vctrs::new_data_frame(df_raw)
tbl <- tibble::new_tibble(df, nrow = nrow(df))

I think ideally the only time I would ever specify nrow is when I want to create a tibble with 0 cols but N rows.

[hadley]

@DavisVaughan shouldn't that start with .row_names_info(x, 2L) in order to work correctly when passed a 0-col data frame?

[DavisVaughan]

While we are thinking about the constructor, I also think x should default to list() like new_data_frame(). With the combination of an always optional nrow and an optional x we'd be able to call new_tibble() with 0 inputs to get a 0 col 0 row tibble, which to me is a very nice property for a constructor

[DavisVaughan]

Ah, yes it should - one more reason to not make me compute this manually 😛

[lionel-]

oh I didn't know new_data_frame() defaults to list(). I think it should default to data.frame(), and nrow should be a compulsory argument when the input is a list to gracefully handle the zero-column case when the list is generated programmatically.