Penguins example looks funny about datamations HOT 15 OPEN

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

from datamations.

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

from datamations.

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

from datamations.

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.

from datamations.

@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.

from datamations.

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

from datamations.

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

from datamations.

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

from datamations.

@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

from datamations.

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

from datamations.

@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.

from datamations.

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

from datamations.

from datamations.

@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!

from datamations.

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

from datamations.