GenieFramework / Genie.jl

🧞The highly productive Julia web framework
https://genieframework.com
MIT License
2.25k stars 188 forks source link

Stipple chart incomplete initialization #452

Closed jeremiedb closed 2 years ago

jeremiedb commented 2 years ago

When trying a minimal Stipple demo, chart seems not to be fully initialized when first accessing the page of refreshing it. Notably, the y axis doesn't adapt to the data range, the x axis labels don't display and the height option isn't taken into account.

At startup or following page refresh: image

After pushing the UPDATE button: image

It can be noted that the chart initialization code and the one triggered on the update button is identical. On other tests, it took 2 refreshes for all the plot options to be properly rendered.

App code is: https://github.com/jeremiedb/GenieExperiments.jl/blob/test-dir/app/resources/histo/HistoController.jl

Latest versions were used along Julia 1.6 :

(GenieExperiments) pkg> st
     Project GenieExperiments v0.1.2
      Status `C:\Users\jerem\OneDrive\JuliaComputing\GenieExperiements\Project.toml`
  [c43c736e] Genie v4.5.0
  [6d011eab] Inflector v1.0.1
  [e6f89c97] LoggingExtras v0.4.7
  [739be429] MbedTLS v1.0.3
  [2913bbd2] StatsBase v0.33.13
  [4acbeb90] Stipple v0.18.2
  [30ddb3f0] StippleCharts v0.16.0 `https://github.com/GenieFramework/StippleCharts.jl.git#master`
  [a3c5d34a] StippleUI v0.12.0
  [ade2ca70] Dates
  [56ddb016] Logging
essenciary commented 2 years ago

@jeremiedb let me check, I remember having a similar issue with the project we developed together.

Also, FYI, I'm considering dropping support for StippleCharts/ApexCharts in order to focus our efforts on StipplePlotly/Plotly as it's more powerful (more types of plots) and has better traction and adoption. Not suddenly, but gradually. I think it can be confusing to have 2 plotting libraries - plus we don't have the bandwidth to properly support two libraries.

What do you think? Are you strongly in favour of using StippleCharts/ApexCharts?

jeremiedb commented 2 years ago

It makes much sense to me to focus the development on a single library rather than to spread on two. Going with Plotly also makes sense given the broader adoption, at least in the scientific / data science user base which will likely constitute the initial user base. That will probably stress the comparison with Dash though, so having a smooth integration with the various StippleUI components will likely be expected. A plotting functionality that works well with Shiny is the rendering of static ggplot2. I rapidly tried with Makie.jl, but since save only takes a string path, I only managed to display temporarily saved image file. I guess it should be feasible to directly serve the serialization of the png / svg?

essenciary commented 2 years ago

Great - for now I'll look into fixing this issue, but yes, long term it makes sense to drop it.

Looked into integrating with Makie already but it's a bit complicated as it uses its own JSServe server and we need to make it work with our "wiring". It's certainly doable and on the roadmap but I probably need to talk to the Makie team to understand how to do it.

jeremiedb commented 2 years ago

I just tried a small demo conversion from Apex to Plotly, and it resulted in lighter synthax, plus tweaking some of the plot options seemed easier, maybe from my familiarity with Plotly or its more straightforward API. In short, I'm on board moving on with Plotly.

Regarding the rendering of Makie plots, I may very well misunderstood key notions, but I thought that it would be possible for the server to send the png as a raw stream, which would be properly understood from the MIME specification.

So something like:

p = scatter(...)
io = IOBuffer()
save(io, p)
model.plot[] = io.data()

I roughly get this from looking at: https://github.com/JuliaPlots/Makie.jl/blob/cb9087ee9055313260611e7eafa8e7a0a0184024/GLMakie/src/display.jl#L40. I'll try taking a look at how Shiny handles the display of ggplot2 as static images.

essenciary commented 2 years ago

Still struggling to get the WhatsYourNumber demo to run on JuliaHub. Renaming the repo did the trick but then it looked like JuliaHub does not support the Julia 1.7 manifest format (v2). So then I needed to go back to Julia 1.6 :-/

Finally, the app started and indeed I have run into the HOST issue myself. At first sight it looks like the ENV is not initialised in the app. This would explain why the environment is dev (Genie sets it to dev by default if no value is set). But then a few lines down it fails because no HOST is set in ENV.

Additionally, the params part is quite fragile - ex if you pass GENIE_ENV as "prod" with the quotes, the launcher seems to crash completely, resulting in a different thing running.

essenciary commented 2 years ago

Re Makie, doesn't it support interactive plots? Cause that's the goal - images are a good fallback, but the current Stipple plotting libraries support interactive charts (labels on hover, zoom, etc)

essenciary commented 2 years ago

I managed to get the app to start by passing HOST as 0.0.0.0 but it looks like the routes are not loaded (I just get 404 on '/'). So something is off with loading the app. I'll keep looking.

jeremiedb commented 2 years ago

Have you tried launching this app: https://github.com/jeremiedb/WhatsYourNumber? There's been a fix made to bootstrap.jl as mentionned in #450 which makes the App working fine on JuliaHub now. I'm taking note regarding the parameters input being sensitive to quotation characters ("").

essenciary commented 2 years ago

@jeremiedb Can we close this? Technically the StippleCharts issue is still present but since you moved to StipplePlotly and the future of StippleCharts is TBD, I can open an issue on that repo.

jeremiedb commented 2 years ago

@essenciary Sorry to bring back the topic, but I just made a migration of an Iris dashboard to Plotly to serve as a tutorial basis and still got that initialization issue.

I you try launching: https://github.com/jeremiedb/GenieExperiments.jl you'll see that the 2 plots are empty. Something intriguing is that the table at the bottom shows actual value for the Cluster column (not the initial 0s), so the compute_clusters! line https://github.com/jeremiedb/GenieExperiments.jl/blob/f9b9a1f7c3afb60ca111c321f94cdbd555d47985/app/resources/dashboards/DashboardsController.jl#L72 is indeed called when entering the app, yet the markers don't show in the plots.

One of the toggles or variable selector need to be modified for the plot to render properly.

jeremiedb commented 2 years ago

@essenciary Following some tests, it appears that the failed plot initialization came with Stipple v0.19. When moving back from Stipple v0.19.2 to v0.18.3, plots are are initialized as expected. I noticed some refactor concerning channels with v0.19, but couldn't identify how it would impact plots.

essenciary commented 2 years ago

Thanks @jeremiedb let me check

essenciary commented 2 years ago

@jeremiedb Indeed, v0.19 introduces many enhancements, some of them breaking. Among many features, we switched to a more secure and performant data sync model where the state is private per user/session by default (vs the previously shared state by default).

As a result, on page load, every user/client gets the default state in JS (which also has the added benefit that the default state can now be cached and served statically via a CDN). This is indicated by the fact that we use Stipple.init(IrisModel) (with the type) instead of the previous Stipple.init(IrisModel()).

In other words, on the initial request, the client gets the result of Stipple.init(IrisModel) -- which creates a default state based on IrisModel() - which has empty cluster data.

iris_plot_data::R{Vector{PlotData}} = PlotData[]   # PlotSeries is structure used to store data
cluster_plot_data::R{Vector{PlotData}} = PlotData[]`

You then proceed to alter the model via

model.iris_plot_data[] = plot_data(:Species, model)
compute_clusters!(model)

but these changes don't get synced as the code is run too early, before the socket connection and the reactive properties are configured.

The new workflow (which mirrors the logic of JS apps) is to listen to the isready event provided by Stipple and push the state there. Alternatively, you could also push the state on page rendering/load by setting iris_plot_data and cluster_plot_data to the actual values instead of empty placeholders. This would have the downside of sending more data in the initial download -- but has the possible upside of rendering the plots even if the socket connection fails.

Thinking about this, I think what we need to do, is to try and capture data pushes that occur before isready and push them automatically when isready is triggered. Might be complicated though...

I've sent a PR that implements the isready logic - it's a minor change. Also, I've made some edits in the views rendering, reflecting recent improvements in terms or rendering performance.

jeremiedb commented 2 years ago

Thanks for these clarifications, I hope to get the tutorial added soon on JuliaHub! Regarding the @iif(:isready) component, I noticed it was put outside of the vector of page elements:

page(model, class = "",  ..., [row[...], row[...]], @iif(:isready))

I saw that the page would display info related to @iif(:isready) is it was included in the vector of components, but I was a bit puzzled about the handling behavior when everything is passed as a vector vs as seperate arguments? Anyhow, that's more for my personal info, so I'm closing.

essenciary commented 2 years ago

Thanks @jeremiedb

I don't understand your point about @iif.