Skip to content
This repository has been archived by the owner on Feb 9, 2024. It is now read-only.

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

Open
ttzhou opened this issue May 25, 2018 · 1 comment

Comments

@ttzhou
Copy link

ttzhou commented May 25, 2018

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
Copy link
Author

ttzhou commented Jun 15, 2018

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?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant