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

type_sum methods ignored for atomic vector based objects in a list column #321

Closed
rundel opened this issue May 17, 2021 · 5 comments · Fixed by #349
Closed

type_sum methods ignored for atomic vector based objects in a list column #321

rundel opened this issue May 17, 2021 · 5 comments · Fixed by #349
Assignees
Milestone

Comments

@rundel
Copy link

rundel commented May 17, 2021

In a recent change to pillar atomic vector based objects are no longer using type_sum methods when printing in a column, see the reprex below for an example:

---
author: rundel
date: 2021-05-17
output: "reprex::reprex\\_document"
title: nerdy-ram\_reprex.R
---

``` r
---
author: rundel
date: 2021-05-17
output: "reprex::reprex\\_document"
title: choky-barb\_reprex.R
---

``` r
library(pillar)

type_sum.list_class = function(x) {
  "list obj"
}

type_sum.atomic_class = function(x) {
  "atomic obj"
}

a = structure(list(1:10), class = "list_class")
b = structure(1:10, class = "atomic_class")
x = structure(
  list(a, b),
  class = c("container", "list")
)

tibble::tibble(x = x)
#> # A tibble: 2 x 1
#>   x              
#>   <containr>     
#> 1 <list obj>     
#> 2 <atmc_cls [10]>

Previously the behavior was that the 2nd row would have contained <atomic obj>.

@rundel rundel changed the title type_sum methods ignored for atomic vector based classes type_sum methods ignored for atomic vector based objects in a list column May 17, 2021
@krlmlr
Copy link
Member

krlmlr commented May 21, 2021

Thanks. It works if we override vec_ptype_abbr(), which is called by the default method of type_sum() . I don't know yet when this was introduced.

What is your use case? Do you think we need to fix this here?

library(pillar)
library(vctrs)

vec_ptype_abbr.list_class <- function(x) {
  "list obj"
}

vec_ptype_abbr.atomic_class <- function(x) {
  "atomic obj"
}

a <- structure(list(1:10), class = "list_class")
b <- structure(1:10, class = "atomic_class")
x <- structure(
  list(a, b),
  class = c("container", "list")
)

tibble::tibble(x = x)
#> # A tibble: 2 x 1
#>   x                
#>   <containr>       
#> 1 <list obj>       
#> 2 <atomic obj [10]>

Created on 2021-05-21 by the reprex package (v2.0.0)

@rundel
Copy link
Author

rundel commented May 21, 2021

I am using it in parsermd when displaying the ast in a tibble format (as a column). The overall ast has a custom s3 class + list and then each node has their own S3 classes, all of these have their own specialization of type_sum. One of the node types was just a character vector of text lines to keep things simple, this is the failing case for me currently.

This doesn't break anything critical for me, but it introduces an undesirable display glitch (and was causing snapshot tests to fail).

@krlmlr krlmlr added this to the 1.6.2 milestone May 24, 2021
@krlmlr krlmlr self-assigned this May 24, 2021
@krlmlr
Copy link
Member

krlmlr commented Jul 26, 2021

Introduced in cd3b4e7, pillar 1.6.1 .

@krlmlr
Copy link
Member

krlmlr commented Jul 26, 2021

Making it work with type_sum() again requires #250, which will take quite some time because it affects downstream CRAN packages. Please implement vec_ptype_abbr().

krlmlr added a commit that referenced this issue Jul 26, 2021
- `obj_sum()` no longer calls `type_sum()` for vectors since pillar v1.6.1, this is now documented (#321).
@github-actions
Copy link
Contributor

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue and link to this old issue if necessary.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants