Penguins example looks funny

[jhofman]

Here's another one that looks strange, the bars are misaligned, there are missing grid panels, and the points go beyond the y-axis range

library(datamations)
library(palmerpenguins)

penguins <- palmerpenguins::penguins

"penguins %>%
  group_by(species, island, sex) %>%
  summarize(mean = mean(bill_length_mm, na.rm = TRUE))" %>%
  datamation_sanddance()

https://user-images.githubusercontent.com/79563/161067367-6c36d774-51fa-4916-aa93-78989acf16a8.mov

Any idea what's up and how to fix this?

(this was run after a fresh devtools::install_github("microsoft/datamations") of datamations this morning)

[jhofman]

this might have to do with facet sorting in the spec?

[giorgi-ghviniashvili]

@willdebras can you remove facet.column.sort and facet.row.sort array on R side? That causes the issue.

[willdebras]

https://user-images.githubusercontent.com/37971596/161149578-45794e2f-9c5a-4ba9-9017-cfda00f9036b.mp4

Removing these sort calls fixes this. It looks like it doesn't introduce issues in other places, so I am going to just commit this to main.

[giorgi-ghviniashvili]

@willdebras that's great. Could you please also make sure that the y axis titles are not bigger than the facet height? Instead of mean(bill_length_mm), use ["mean", "bill_length_mm"] and it will be fully fixed.

[willdebras]

Yep :) It's like this in main now after merge from yesterday, returning ["mean of", "variablename"]. Just hadn't pulled those changes yet.

[jhofman]

almost there---sorting of the rows changes part way through, see here:

https://user-images.githubusercontent.com/79563/161275525-85d3d67c-c734-48fa-a51d-1fb76bf8665d.mov

[willdebras]

Might need to revert this, as it keeps the sorting consistent with group_by and mutate frames https://github.com/microsoft/datamations/commit/1ac1cf50f8ffd673a29e93dd7c88ac0dbad3f88c

[giorgi-ghviniashvili]

@willdebras alright, right now, I brought back the sorting and trying to fix the other way around.

[giorgi-ghviniashvili]

@jhofman @willdebras I found out that vega-lite facet sort does not work at all and that's why it has this issues.

The issue: https://github.com/vega/vega-lite/issues/5937

The image:

So I guess it is better to remove sorting, unless we hack that too :D

Play with this vega example

[willdebras]

What's your recommendation as far as specs, @giorgi-ghviniashvili? Should I remove all sorting? Removing the facet sorting from summarize spec generation de-syncs it from the group_by(). It seems if we sort all, we get the weird grid. If we unsort in this step, we get the weird shifts from the group_by().

[giorgi-ghviniashvili]

@willdebras this pr #169 removes sorting from generateGrid (group_by()), so I think it worths testing and if that works, then we are good to go. Since the facet sorting does not work in vega-lite at all, we don't need them. Please test that PR and let me know if that syncs up with the other steps.

[willdebras]

Thanks! I will test in the morning my time and confirm all is good.

[jhofman]

hi all—just wanted to see where this landed?

On Fri, Apr 1, 2022 at 11:15 PM Will Bonnell @.***> wrote:

Thanks! I will test in the morning my time and confirm all is good.

— Reply to this email directly, view it on GitHub https://github.com/microsoft/datamations/issues/163#issuecomment-1086531742, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAATNS6DGYPROFPA5GVNCWLVC63VVANCNFSM5SFNBQVQ . You are receiving this because you were mentioned.Message ID: @.***>

[willdebras]

@giorgi-ghviniashvili's PR looks like it works fine from my testing, but I want to spend another 30-45 minutes today or tomorrow testing that it doesn't cause issues with the new mutate stuff, i.e. that we don't accidentally trigger a rearrange with the grouped mutations. going into a faceted summarize. I can confirm tomorrow that that is good to merge and then we are all set!

[willdebras]

I've tested this a bit more and it looks good to go to merge Giorgi's PR. This should be fixed now!