JohnCoene / echarts4r

🐳 ECharts 5 for R
http://echarts4r.john-coene.com/
Other
585 stars 82 forks source link

Grouping column can't have NA #571

Open beatrizmilz opened 9 months ago

beatrizmilz commented 9 months ago

Hi! First of all, thanks for this awesome package.

I found a potential bug (or at least I think would be good to have a message or warning to let people know about this behavior).

When I plot a grouped dataset that has NA in the grouping column, the plot turns out blank.

In order for it to work, I need to explicitly remove the NA's before grouping the dataset.

# does not work
palmerpenguins::penguins |>
  dplyr::group_by(sex) |>
  echarts4r::e_chart(x = body_mass_g) |>
  echarts4r::e_scatter(serie = flipper_length_mm)

# works
palmerpenguins::penguins |>
  tidyr::drop_na(sex) |>
  dplyr::group_by(sex) |>
  echarts4r::e_chart(x = body_mass_g) |>
  echarts4r::e_scatter(serie = bill_depth_mm)

I installed the package from CRAN. Package version:

> packageVersion("echarts4r")
[1] ‘0.4.5’
munoztd0 commented 9 months ago

Well that seems "normal" to me, it's just common practice to let the user choose what to do with the NAs in grouping variables. Maybe an error or warning would be needed

# with Explicit NA as level palmerpenguins::penguins |> dplyr::mutate(sex = forcats::fct_na_value_to_level(sex, "NA") ) |> # or calling it "Missing" dplyr::group_by(sex) |> echarts4r::e_chart(x = body_mass_g) |> echarts4r::e_scatter(serie = flipper_length_mm)

image