ggvis icon indicating copy to clipboard operation
ggvis copied to clipboard

layer_bars does not work if `x` and `y` are not initially specified in `ggvis` call

Open ttzhou opened this issue 7 years ago • 1 comments

The following does not work:

library(magrittr)
library(ggvis)

mtcars %>%
ggvis() %>%
group_by(gear) %>%
layer_bars(
    x = ~factor(cyl),
    y = ~mpg,
    fill = ~factor(gear),
    width = 1
) %>% 
scale_ordinal('x')

Error is: Error in UseMethod("prop_label") : no applicable method for 'prop_label' applied to an object of class "NULL"

But the following does:

library(magrittr)
library(ggvis)

mtcars %>%
ggvis( #specifying the x and y here now, instead of later
    x = ~factor(cyl),
    y = ~mpg
) %>%
group_by(gear) %>%
layer_bars(
    fill = ~factor(gear),
    width = 1
) %>% 
scale_ordinal('x')

Even the following works:

library(magrittr)
library(ggvis)

mtcars %>%
ggvis(
    x = ~hp,
    y = ~mpg
) %>%
group_by(gear) %>%
layer_bars(
    x = ~factor(cyl),
    y = ~mpg,
    fill = ~factor(gear),
    width = 1
) %>% 
scale_ordinal('x')```

i.e. layer_bars works only if x and y are initially set in the initial ggvis call.

The source code for layer_bars seems to ask for 'x.update', which I'm guessing only works if x is initially defined.

This does not seem to be the case for layer_points, i.e.

library(magrittr)
library(ggvis)

mtcars %>%
ggvis() %>%
layer_points(
    x = ~factor(gear),
    y = ~mpg
) %>% 
scale_ordinal('x')

works fine.

In my opinion, this should be made consistent... if some marks allow for empty initial values for props, I think all of them should?

Also: thank you, to the entire team, for making such a great contribution to the data community. I know this isn't high on your priority list right now, but there are definitely people out there who love ggvis and are hoping for further development.

ttzhou avatar May 25 '18 13:05 ttzhou

In layer_bars:

function (vis, ..., stack = TRUE, width = NULL) 
{
    new_props <- merge_props(cur_props(vis), props(...))
    check_unsupported_props(new_props, c("x", "y", "x2", "y2"), 
        c("enter", "exit", "hover"), "layer_bars")
    x_var <- find_prop_var(new_props, "x.update")
    discrete_x <- prop_countable(cur_data(vis), new_props$x.update)
    vis <- set_scale_label(vis, "x", prop_label(**cur_props**(vis)$x.update))
    ...
}

Would setting cur_props(vis) to new_props(vis) rectify this?

ttzhou avatar Jun 15 '18 20:06 ttzhou