VEuPathDB / web-eda

Web browser code for EDA-based applications
Apache License 2.0
0 stars 0 forks source link

Refactor visualization implementations #580

Open bobular opened 2 years ago

bobular commented 2 years ago

All of our visualizations have the same basic layout and data flow. This should be encapsulated in a component that handles the layout and data flow.

Overview

All visualizations have the same basic layout, as illustrated in the image below: image

The Heading section is provided by the VisualizationsContainer component, so we can ignore that. The other two sections are handled by individual visualization components. The goal is to create a component that can render these two sections with a minimal amount of configuration.

The Inputs section is typically rendered using the InputVariables component. In most visualizations, the specification for the inputs can be statically declared. However, there are increasingly more cases where this can be dynamic, either based on inputs provided by the App (computed variables), or based on the selection of other variables (see Y-aggregation options for line plots).

The Plot and controls section generally has the same structure across all visualizations: a plot and controls on the left, and legend and summary elements on the right.

Proposal

Implement a StandardVisualization component. This component will be responsible for rendering the sections Inputs and Plot and controls sections detailed above. Each visualization will specify how to render inputs, and how to render the plot and controls. StandardVisualization will wire the pieces together, handling data flow from inputs to plots, and will render the layout, included the summary elements.

We will likely want to incorporate some changes desirable for better handling integration with Apps. For example, an App instance can provide one or more variables to the visualization. This should be handled in a generalized way.

TODOs

Additional details (copied from confluence doc)

A collection of thoughts about the viz refactor and how apps may want to interact with visualizations in web-eda

Apps may bring their own computed variables, opinions about plot options, and axes titles into a visualization. Below is a compiled list of all the overrides I currently know about or expect based on our planned apps. Note that the following is a union of all the app declarations, and each may not be applicable to all apps.

Known overrides Variables:

Plot metadata:

Plot modes:

Others:

bobular commented 2 years ago

One issue to think about at the same time is to try to avoid making many checks on data consistency like in the fixes for the ticket below, when inputs have been changed but the default axis ranges have not yet been returned from the back end (or something like that).

https://github.com/VEuPathDB/web-eda/issues/1003

dmfalke commented 2 years ago

Today, we discussed the need to control the disabled attribute of the showMissingness toggle. This toggle is based on the data response for the viz.

I had suggested the Plot and controls section would handle getting data, but doing so means the StandardVisualization component does not have access to the data.

I think another approach we can take is that StandardVisualization takes a callback to get data, as a prop. This callback would take the current visualization config object as an argument. StandardVisualization could then call this callback when the configuration changes, allowing it to control the state of the toggle based on the response. Plot and controls would simply be responsible for updating the visualization configuration.

One issue to think about at the same time is to try to avoid making many checks on data consistency like in the fixes for the ticket below, when inputs have been changed but the default axis ranges have not yet been returned from the back end (or something like that).

I think this can be handled by waiting to update the viz config until after the data is fetched. Thus, Plot and controls we get a callback to update the vizConfig, and StandardVisualization would orchestrate fetching data and updating the config.

bobular commented 2 years ago

Cool. Sounds even more important to split the vizConfig object into two parts: client-side and server-side configs. Only changes in the latter trigger new data requests. Trouble is, this is likely a breaking non-backwards compatible change.

ETA: though us changing a configuration from server to client (or vice-versa) - which might happen in the future - shouldn't be a breaking change either (but it would be under my suggestion)

asizemore commented 2 years ago

Could StandardVisualization allow for props like title, xAxisLabel, includePlotModes? Apps will often define variables, but also have information that could be an axis label or that affects if scatter plot trend lines, for example, should be allowed

bobular commented 2 years ago

It's possible that the ShowMissingness toggle could be provided as a CustomInputSection?

Not sure if that helps or hinders!

dmfalke commented 2 years ago

Cool. Sounds even more important to split the vizConfig object into two parts: client-side and server-side configs. Only changes in the latter trigger new data requests. Trouble is, this is likely a breaking non-backwards compatible change.

ETA: though us changing a configuration from server to client (or vice-versa) - which might happen in the future - shouldn't be a breaking change either (but it would be under my suggestion)

@bobular I don't think we need to split things out like that. Part of the API for that callback could be that if undefined is returned, we just use the current value, so we wouldn't make a new request every time. Or something like that.

dmfalke commented 2 years ago

Could StandardVisualization allow for props like title, xAxisLabel, includePlotModes? Apps will often define variables, but also have information that could be an axis label or that affects if scatter plot trend lines, for example, should be allowed

@asizemore I think these might be visualization-specific props. I think it will become more clear where these should go, once I've added that infrastructure to this component. I'm hoping to get to that tomorrow (I have other priorities today).

dmfalke commented 2 years ago

It's possible that the ShowMissingness toggle could be provided as a CustomInputSection?

Not sure if that helps or hinders!

@bobular I'm not convinced this would be a good thing. If this is a standard option, then we should keep it as-is and figure out the best way to determine if it is enabled. To me, the way to do that is to ensure all viz responses include the same set of properties describing missingness. I think this is already the case, we just need to encode that in our types (i.e., make a base response type) so that we can access it in a visualization-generic way.

asizemore commented 2 years ago

small note: Instead of "plot title" we can just have "plot info". Plot info is more descriptive of the goal anyways. The point is that sometimes the computation returns interesting information that would be helpful to the user when they read the viz. That info does not have to go in the plot title, but should go somewhere. One option for implementation was added in #1173

asizemore commented 2 years ago

Should we be thinking about axes and colormaps here, too? If we do full screen eda with the map, where the map is the main plot but it has smaller subordinate plots that share the same colormaps, would it be useful to be able to have the main viz tell the small viz what their colormap should be? To be fair i have no idea how that main-viz-with-component-viz would all work but throwing it out there in case it helps with the current refactor!

dmfalke commented 2 years ago

Should we be thinking about axes and colormaps here, too? If we do full screen eda with the map, where the map is the main plot but it has smaller subordinate plots that share the same colormaps, would it be useful to be able to have the main viz tell the small viz what their colormap should be? To be fair i have no idea how that main-viz-with-component-viz would all work but throwing it out there in case it helps with the current refactor!

It's not clear to me how the map will be implemented. I was thinking it could be an app, since they are already containers for visualizations. It is also natural for a viz to provide specific variables (such as overlay, in the case of the map).

ETA I'm not totally grokking where colormap fits in.

asizemore commented 2 years ago

I was thinking it could be an app, since they are already containers for visualizations

That makes sense! And also how i expect scRNASeq would work (very down the line, to be fair) as well.

The colormap part would be the app determining what colormap to use for all of its vizs. So if the main viz (map, let's say), is coloring points with a particular colormap, then we want all the other vizs to also use that colormap (see page 7 of Bob's mockups).

But perhaps these colormap things are all too far in the future for us to worry about in this refactor?

dmfalke commented 2 years ago

It's good to think about now, for sure! What's becoming clear is that we need to allow apps to specify these things dynamically, based on the app instance's configuration. The challenge we face right now is determining an interface between an app instance and its visualizations such that we can specify these things in a streamlined way, for any viz. I suppose that is the most critical task, at the moment.

asizemore commented 2 years ago

Forever in progress, but dropping another confluence doc full of thoughts here