Simplify dplyr_reconstruct()

[DavisVaughan]

Closes #5158

This drastically simplifies dplyr_reconstruct.data.frame() by taking an approach similar to dplyr_col_modify.data.frame(), which just restores the attributes directly.

I've also pulled over the tests from #5158, but I think the attributes one might have been backwards. I think we care about keeping the attributes from template, not data.

The job of the data frame method of dplyr_reconstruct() is now very simple and well defined. It keeps all attributes of template except:

• names in case columns were modified
• row.names in case rows were modified

There was some code about the .drop attribute in the old dplyr_reconstruct.data.frame(), but I don't think it belonged there because we have a grouped_df method as well. It also wouldn't have worked right because the way to get to the drop attribute is through the group data of the data frame. It's not on the data frame directly.

[hadley]

I think this also fixes #5132?

[DavisVaughan]

That issue links to #4617, which has this example:

library(dplyr, warn.conflicts = FALSE)

df1 <- tibble(
  a = 1:2, 
  b = 2:1
)

df2 <- tibble(
  a = structure(1:2, foo = "bar"),
  c = 2:1
)

left_join(df1, df2, by = "a")$a
#> [1] 1 2
left_join(df2, df1, by = "a")$a
#> [1] 1 2
#> attr(,"foo")
#> [1] "bar"

That behavior isn't touched by this PR.

But honestly I sort of feel like this behavior is right. You get the attributes on the columns of the first input?

[DavisVaughan]

With dplyr_col_modify(), dplyr_row_slice(), and dplyr_reconstruct() already in place, I don't currently see any cases where we aren't preserving free floating attributes. I think we are doing the right thing now? Which implies that issue #5132 can be closed anyways.

[krlmlr]

I think this would benefit from a lightweight C function that walks the pairlist, replaces in place and appends if necessary.

[hadley]

Yeah, that might be a useful helper for vctrs or similar. It doesn't need to block this PR.

[hadley]

Thanks @DavisVaughan!