search

microsoft / datamations

https://microsoft.github.io/datamations/
Other
67 stars 14 forks source link

Shiny app can't handle changing data/groups more than once without freezing #60

Closed sharlagelfand closed 3 years ago

sharlagelfand commented 3 years ago

Just to keep track of this so I dig into it... I'm not sure if it's an issue with the way the Shiny app is laid out or how the specs are generated/animated, but will look into it.

sharlagelfand commented 3 years ago

@giorgi-ghviniashvili I'm trying to track down why this is happening, in the app if you hit "Go", then "Go" again, there is this error in the console:

vlSpec.meta is undefined

I'm not sure what's making it freeze but could you try it out and look into that error? Thank you!

giorgi-ghviniashvili commented 3 years ago

@sharlagelfand could you plz record a video?

sharlagelfand commented 3 years ago

https://user-images.githubusercontent.com/15895337/119733221-340aa380-be47-11eb-9085-63e7f0f39307.mov

giorgi-ghviniashvili commented 3 years ago

@sharlagelfand latest code from error-bars will fix this as well.

sharlagelfand commented 3 years ago

thanks @giorgi-ghviniashvili! the latest app is here, the animation plays eventually but the first bit is frozen/choppy and the error persists

https://user-images.githubusercontent.com/15895337/119829117-e8004300-bec8-11eb-9fcd-b8f83676dea1.mov

giorgi-ghviniashvili commented 3 years ago

Something is strange here, we need to debug it .

giorgi-ghviniashvili commented 3 years ago

@sharlagelfand , I see that init function is called multiple times after click "Go", can you see why it happens? It must only call it once..

sharlagelfand commented 3 years ago

@giorgi-ghviniashvili yeah i can see it is called once the first time, then twice the second time "go" is clicked.... I'll see if I can figure out why

giorgi-ghviniashvili commented 3 years ago

@sharlagelfand if you can't figure it out, I will just use flag variable: initializing: true | false and will ignore all subsequent init calls while initializing. Just lmk.

sharlagelfand commented 3 years ago

Thanks @giorgi-ghviniashvili - the latest app is here, as far as I can tell on my end init is not being called multiple times but it's still freezing/erroring:

https://user-images.githubusercontent.com/15895337/120211945-6decf880-c1ff-11eb-9ee8-d1da380b3da4.mov

Maybe let's try adding that flag to ignore subsequent init calls? Just checking, this will only ignore them "while initializing", but will allow for calling again via pressing "go" / changing settings etc, right?

giorgi-ghviniashvili commented 3 years ago

Just checking, this will only ignore them "while initializing", but will allow for calling again via pressing "go" / changing settings etc, right?

Of course :) It will just ensure that init function will complete.

sharlagelfand commented 3 years ago

Ah shoot, different error now when you hit "go" twice on the updated app:

vegaLiteSpecs[i] is undefined

giorgi-ghviniashvili commented 3 years ago

@sharlagelfand good catch, please update it.

sharlagelfand commented 3 years ago

Thanks @giorgi-ghviniashvili!! It definitely does not freeze now 🎉 Updated here

It seems like maybe something is "off by 1" in terms of the animation shown - maybe having to do with the initialization?

The first time I click "go" it's the default settings - group by Degree, calculate mean Salary - and this works great. The second time (~18 seconds), I group by Degree and Work, but the animation still shows just grouped by Degree. The third time (~40 seconds), I group by Degree and Work, and calculate median Salary, but the animation shows grouped by Degree and Work, but the mean - so it looks like any time after the first click, it uses the previous iterations specs?

https://user-images.githubusercontent.com/15895337/120237402-2039b580-c22a-11eb-9ba4-c6a81cb8ad45.mov

giorgi-ghviniashvili commented 3 years ago

@sharlagelfand that's strange. I added console.log statements to print out passed specs. Please update it.

sharlagelfand commented 3 years ago

@giorgi-ghviniashvili updated app is here - the log seems to show the lag in specs from what I can tell!

giorgi-ghviniashvili commented 3 years ago

@sharlagelfand init function is called twice after second click on "GO":

And because initializing flag, second call (with correct specs) is ignored.

So this means that there is an issue on Shiny side, it calls init with old specs.. Please check this, it MUST call init only once.

See video: https://user-images.githubusercontent.com/6615532/120631993-8e27ec00-c479-11eb-8f3d-550f030eff0c.mov

sharlagelfand commented 3 years ago

Ahh super annoying, it should only be calling once... but let me debug further. Thanks for looking into this!

sharlagelfand commented 3 years ago

Just to update, I have sorted this out 🙌🏻 🥳 Still working out some fiddly bits for #45 but I'll push an updated version of the app with both features / fixes once that's sorted.

sharlagelfand commented 3 years ago

Updated app is here with this fix! 🤞🏻

sharlagelfand commented 3 years ago

This seems all good, finally, so closing now!