Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SIP-6] [Embeddable Charts] Migrate visualization-specific code to js plugin #5692

Closed
kristw opened this issue Aug 21, 2018 · 13 comments
Closed
Labels
sip Superset Improvement Proposal

Comments

@kristw
Copy link
Contributor

kristw commented Aug 21, 2018

Continued from the discussion in [SIP-5] #5680

There is a decent amount of work to be done to make the visualization plugins are truly independent from the core superset.

Q1: What need to be converted to JS?

@mistercrunch : To make this frontend visualization plugin possible, we'll need to figure out how to get rid of the visualization-specific backend code in viz.py over time, to make sure that visualizations plugins are fully defined as Javascript.

  • One easy step forward is to move the backend's query_obj code, which takes form_data as input and returns a query object (simple logic).

  • The get_data method is probably harder to migrate to the front-end (takes a dataframe and returns nested objects), but it can probably be generalized quite a bit, so that it's not visualization-specific as much as ordering pandas-like operations on the result set.

Q2: Where these new front-end methods should live?

@mistercrunch : I'm guessing you were thinking about export the adaptor and using this as an the interface between chart and the visualization plugin. Now I'm thinking we may want to export a more complex object.

export const visualization = {
  // Adapter from [SIP-5]
  adaptor, 
  // Convert formData into queryable format
  queryGenerator: (formData) => generateQuery(formData),
  // Convert formData into post-query operations 
  dataframeOperations: (formData) => (
    ['pivot_table',  { cols: [formData.cols], rows: formData.groupby }]
  ),
};

Q3: How to actually implement these two parts?

@mistercrunch : I'm not sure how to model the query vs dataframe operations, should it be incorporate into the query, or made into its own thing. Clearly on the backend we need two abstractions as one is used to interact with connectors (querying) and the other applies to all connectors (dataframe transformation).

@john-bodley : I'm also perplexed as to how to best generalize this. It seems a first step is to map arbitrary data (via a query) into a somewhat generic (potentially visualization aware) form.

@mistercrunch : I also wonder whether we could expose most of the pandas API from the frontend by doing a bit of magic, where the dataframeOperations above would somehow do something like getattr(df, command)(**props) on the backend.

@kristw
Copy link
Contributor Author

kristw commented Aug 22, 2018

DECISION: Out-of-scope


After syncing offline with @john-bodley , it seems like we need to think more about

What kind of UI we have in mind on the explore page?

A. Users need to formulate query to get tabular data first, then map the columns from intermediate table to the encoding channels (color, size, length, position, etc.)

B. The controls list encoding channels, which users can put columns/metrics/aggregates from datasource and our engine translates this back into database query + post-query transformation.

What operations should be done on back-end/front-end?

For example, group by and aggregates (count,sum,avg,etc.) should leverage database.
Transforming table into tree nodes can be done in js.
What about multiple time series? The query engine needs to know that it is dealing with time series so it will also group by time in the query.
[{ name, time, value }]

@mistercrunch
Copy link
Member

I was thinking solely about the shortest path to making visualizations as JS plugins, without redesigning the explore user interface.

I'm very supportive of redesigning the explore interface, but we may want to think of this as a different step or project altogether. Both these efforts are sizable on their own.

Happy to go either way, but personally I prefer smaller chunks of work.

@victornoel
Copy link
Contributor

Also if you don't redesign the explore interface now, the "charts as js plugin" feature can be integrated sooner and people will start using it, which should give lots of user inputs about how the explore interface should be redesigned.

@kristw kristw changed the title [SIP-6] [Embeddable Charts] Migrate visualization-specific back-end code from viz.py to js plugin [SIP-6] [Embeddable Charts] Migrate visualization-specific code to js plugin Aug 23, 2018
@kristw
Copy link
Contributor Author

kristw commented Aug 23, 2018

Agree about about smaller chunk of work. I exclude the explore interface redesign from this SIP and evaluate the work for extracting without changing UI in the diagram below.

Here is the state of the app today.

viscodestructure 001

@kristw
Copy link
Contributor Author

kristw commented Aug 23, 2018

DEPRECATED: Please see more updated structure below.


[Proposal] Each chart plugin package includes

My Chart
  |--formDataSchema.js // #3
  |--dataSchema.js // #2
  |--Adaptor.jsx  // #1 + #2
  |--MyChart.jsx // #1
  |--index.js

index.js

export default new ChartPlugin({
  // Can separate into metadata.js
  metadata: {
    key: 'my-chart',
    label: 'MyChart',
    description: 'lorem ipsum dolor',
  },
  // Can separate into buildQuery.js
  buildQuery: (formData) => { ... },  // #3
  loadAdaptor: () => import('./Adaptor.jsx'),  
  // Can separate into controlPanelConfig.js
  controlPanelConfig: ..., // #4
});

@williaster williaster added the sip Superset Improvement Proposal label Aug 24, 2018
@kristw
Copy link
Contributor Author

kristw commented Aug 27, 2018

The types of each field in formDataSchema can also be extended to be more specific than the primitive types, so we can automate some validation and post-processing. Some extra types:

  • dateFormat: can be either string or function. If string, will be converted to function via d3.format and passed to the Chart as function
  • numberFormat: can be either string or function. If string, will be converted to function via d3.format and passed to the Chart as function
  • jsFunction: will be sandboxEval and passed to Chart as function
  • colorPalette: ...

@kristw
Copy link
Contributor Author

kristw commented Aug 27, 2018

The interface of how to install/register components still need to be revised, but they key idea here is each chart will require formDataSchema and dataSchema that informs the implementation of the control panel or the chart itself.
image

@conglei
Copy link
Contributor

conglei commented Aug 31, 2018

Refractor of get_data()

goals

Decoupling all get_data() functions in viz.py with visualization types. To do this, we should:

  • move chart specific logic to visualization frontend code base.
  • create a new field in form_data to specify some standard data processing method. (This is ONLY for some computational heavy process AND cannot be done by query_object).
  • move away the config parameters (e.g., mapbox_token).

Status

Status of moving get_data() logic to frontend (or decouple the logic with visualization type).

PR Visualization Status
#5753 Word cloud merged
#5861 BigNumber under review
Tree Map
Chord Vis
iframe and markup
Horizon Chart
Force-directed Graph
Sunburst
Sankey
Heatmap
Pivot Table
Table
Partition
World Map
Country Map
Histogram
Calendar Heatmap
Parallel Coordinates
Paired t-test
Rose
Time Series Table
MapBox
FilterBox
NVD3

@zhaoyongjie
Copy link
Member

@kristw How is such a cool diagram generated?

@kristw
Copy link
Contributor Author

kristw commented Sep 2, 2018

@zhaoyongjie Drawn manually :)

@kristw
Copy link
Contributor Author

kristw commented Sep 13, 2018

The directory structure for each plugin will look like this. Using WordCloud as an example.

WordCloud
  |--buildQuery.js
  |--transformProps.js
  |--transformData.js
  |--metadata.js
  |
  // for the vanilla js / d3 components
  |--WordCloud.js 
  |--ReactWordCloud.jsx // convert WordCloud.js to react component
  // for the React components
  |--WordCloud.jsx
  |
  // Use temporarily until the plugin system is fully implemented
  |--adaptor.jsx
  // This will replace adaptor.jsx once the plugin system is fully implemented
  |--plugin.js
  |
  // TBD: Control configuration 
  |--controlConfig.js
  |
  // TBD: This could be class for input/output of the transformProps function
  |--formDataSchema // 
  |--dataSchema // 

What is in each file?

1. buildQuery.js

export default function buildQuery(formData) {
  return queryObject;
} 

2. transformProps.js

export default function transformProps({ 
  slice, 
  payload, 
  setControlValue,
}) {
  // Return props that are compatible with <WordCloud />
  // In the future can enforce type for the returned object if we use TypeScript. 
  return props; 
}

Note: In the future, will replace slice with specific fields, such as width, height, or formData and perhaps add type (if we use typescript) for formData

3. transformData.js

Transform props that are based on payload. The logic formerly in get_data will be moved here.
For the first version, it will be called from transformProps.js, but separated into another function can be useful for future optimization.

export default function transformData({ payload, formData }) {
  return props;
}

4. metadata.js

Description of this chart

import thumbnail from './WordCloud.png';

export default new ChartMetadata({
  key: 'word-cloud',
  name: 'WordCloud',
  description: 'Lorem ipsum dolor sit amet',
  thumbnail,
});

5.1 WordCloud.js and ReactWordCloud.jsx

For vanilla components that are not React yet, we have been refactoring to make it convertible

WordCloud.js

export default function(element, props) { ... }

ReactWordCloud.jsx

import reactify from '../../utils/reactify';
export default reactify(WordCloud);

5.2 WordCloud.jsx

For components that are already React component

class WordCloud extends React.Component {
  ...
}
export default WordCloud;

6. adaptor.jsx

Shim to make it work with current Superset app. This file will be deleted after we completely move to the plugin system.

import createAdaptor from '../../utils/createAdaptor';
import WordCloud from './ReactWordCloud';
import transformProps from './transformProps';

// return function(slice, payload, setControlValue) that render the component
export default createAdaptor(WordCloud, transformProps);

7. plugin.js

This is how a chart plugin is defined. It will replace adaptor.jsx.

import buildQuery from './buildQuery';
import transformProps from './transformProps';
import metadata from './metadata';
import WordCloud from './ReactWordCloud';
export default new ChartPlugin({
  key: 'word-cloud',
  metadata,
  buildQuery,
  transformProps,
  Chart: WordCloud,
});

Plugins are installed like this

import wordCloudPlugin from './WordCloud/plugin';
wordCloudPlugin.install();

Then you can use

<SupersetDataProvider type="word-cloud" formData={...}>
  <SuperChart type="word-cloud" xxx={...} />
</SupersetDataProvider>

There is an ongoing work for plugin system in #5882 which explains what happens behind-the-scene in .install(). Please see the PR for more details.


@conglei

@kristw
Copy link
Contributor Author

kristw commented Sep 14, 2018

Until the plugin mechanism is implement, we will be converting the existing chart to conform with the structure above. Example is this PR.
#5893

@stale
Copy link

stale bot commented Apr 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the inactive Inactive for >= 30 days label Apr 10, 2019
@kristw kristw removed inactive Inactive for >= 30 days labels Apr 10, 2019
@kristw kristw closed this as completed Apr 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sip Superset Improvement Proposal
Projects
Status: Implemented / Done
Development

No branches or pull requests

6 participants