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()

Screen.Recording.2022-03-31.at.9.32.06.AM.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]

Viewer.Zoom.2022-03-31.16-07-11.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:

Screen.Recording.2022-04-01.at.9.42.11.AM.mov

[willdebras]

Might need to revert this, as it keeps the sorting consistent with group_by and mutate frames 1ac1cf5

[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: vega/vega-lite#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 <#163 (comment)>, 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!