helgasoft / echarty

Minimal R/Shiny Interface to ECharts.js
https://helgasoft.github.io/echarty/
87 stars 3 forks source link

1.5.0 causes breaks for all data without rownames or non-numeric keys #21

Closed yogat3ch closed 1 year ago

yogat3ch commented 1 year ago

Hi @helgasoft, Thank you for your work on 1.5. We just installed it but were forced to revert because of breaking changes that were introduced in the update. The offending line is here: https://github.com/helgasoft/echarty/blob/07e242e29fcd0668ec4eb06d6f5db7871a30fc2b/R/echarty.R#L175 This breaks echarty for all crosstalk data that uses a non-numeric value as a key. Is there a reason for this check, I don't see why it's necessary?

helgasoft commented 1 year ago

Thank you for pointing this out, the limitation is indeed not justified. Package echarty is now updated to allow non-numeric keys in crosstalk. Example:

# crosstalk with character keys
library(dplyr); library(echarty)
shared_mtcars <- SharedData$new(mtcars |> relocate(wt, mpg))
bscols(
  list(
    filter_checkbox("cyl", "Cylinders", shared_mtcars, ~cyl, inline = TRUE),
    filter_slider("hp", "Horsepower", shared_mtcars, ~hp, width = "100%"),
    filter_select("auto", "Automatic gearbox", shared_mtcars, ~ifelse(am == 0, "Yes", "No"))
  ),
  d3scatter(shared_mtcars, ~wt, ~mpg, ~factor(cyl), width="100%", height=250),
  ec.init(shared_mtcars, xAxis=list(max=6), yAxis=list(max=35), animation=F)
)

If you encounter problems with the latest version, please report in this thread.

yogat3ch commented 1 year ago

That's great new @helgasoft, thanks for removing that check! I'll have to chance to check it out this week and confirm that it's working

yogat3ch commented 1 year ago

I think this has been fixed @helgasoft, thank you.

Im noticing that the grouping variable in the crosstalk data, which in this case is the key column, is being added to the options - series data as an entity named XkeyX. This is causing a bunch of downstream breaks since we process that series data in all sorts of ways and previously the grouping variable nor key showed up in the series data. I don't have time to fully investigate what's going on under the hood with that so I'm just going to revert back to 1.4.5 for now.

If I have time to get back to it in the future I'll open another issue. Thanks for your assistance with this 🙏