feat: Add Encoder #230

kristw · 2019-10-02T08:04:54Z

🏆 Enhancements

Add Encoder to @superset-ui/encodable. An Encoder is a utility class created once per chart type.

It consists of one or more ChannelEncoder
It takes users' definitions of the channels (vega-lite syntax), then resolves ambiguity and produce complete definition and actionable utility functions.

Example Usage

The component developer creates an encoder factory once.

type LineChartEncodingConfig = {
  // channelName: [ChannelType, output type, support multiple fields or not]
  x: ['X', number];
  y: ['Y', number];
  color: ['Color', string];
  tooltip: ['Text', string, 'multiple'];
};

const lineChartEncoderFactory = createEncoderFactory<LineChartEncodingConfig>({
    // This part is manual work to convert the `ChannelType` defined in `LineChartEncodingConfig` above
    // from `type` to `value`. `LineChartEncodingConfig` is the source of truth 
    // and because it is generic for this function, type checking will ensure the only compatible value
    // for channelTypes is exactly like the one below.
    channelTypes: {
      x: 'X',
      y: 'Y',
      color: 'Color',
      tooltip: 'Text',
    },
    // Define default definition for each channel
    defaultEncoding: {
      x: { type: 'quantitative', field: 'x' },
      y: { type: 'quantitative', field: 'y' },
      color: { value: 'black' },
      tooltip: [],
    },
  });

type LineChartEncoding = DeriveEncoding<LineChartEncodingConfig>;

The factory encapsulates the awkward channelTypes and defaultEncoding

which are constants across all Encoder instance of this chart
making it convenient to create a new Encoder from encoding because factory.create(encoding) only needs one argument: encoding.

This is how user-specified encoding may look like.

const encoding: LineChartEncoding = {
  x: { type: 'quantitative', field: 'speed', scale: { domain: [0, 100] } },
  y: { type: 'quantitative', field: 'price' },
  color: { type: 'nominal', field: 'brand' },
  tooltip: [{ field: 'make' }, { field: 'model' }],
};

Then create an Encoder for the incoming encoding and use it to facilitate rendering.

const encoder = lineChartEncoderFactory.create(encoding);
encoder.channels.x.getValueFromDatum({ speed: 100 }); // 100
encoder.channels.x.encodeDatum({ speed: 100 }); // 1
encoder.getGroupBys(); // ['brand', 'make', 'model']

netlify · 2019-10-02T08:11:22Z

Deploy preview for superset-ui ready!

Built with commit 34960fd

https://deploy-preview-230--superset-ui.netlify.com

codecov · 2019-10-02T09:46:43Z

Codecov Report

Merging #230 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #230   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files         138    142    +4     
  Lines        1520   1559   +39     
  Branches      400    406    +6     
=====================================
+ Hits         1520   1559   +39

Impacted Files	Coverage Δ
...set-ui-encodable/src/fillers/completeChannelDef.ts	`100% <ø> (ø)`	⬆️
...ages/superset-ui-encodable/src/encoders/Encoder.ts	`100% <100%> (ø)`
...t-ui-encodable/src/fillers/completeLegendConfig.ts	`100% <100%> (ø)`
...perset-ui-encodable/src/encoders/ChannelEncoder.ts	`100% <100%> (ø)`	⬆️
...-ui-encodable/src/encoders/createEncoderFactory.ts	`100% <100%> (ø)`
...s/superset-ui-encodable/src/utils/mergeEncoding.ts	`100% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e7d7249...34960fd. Read the comment docs.

williaster

LGTM overall, had a couples questions. very cool types!

the vx re-write has made me more appreciative of TS and nuances :)

williaster · 2019-10-03T16:52:09Z

packages/superset-ui-encodable/src/encoders/Encoder.ts

+        channels[name] = definitions.map(
+          (definition, i) =>
+            new ChannelEncoder({
+              channelType: 'Text',


should this come from channelTypes[name] or channelTypes[name][0]? or Text is the only type that has an array? (seems possibly fragile in the future)

williaster · 2019-10-03T16:55:17Z

packages/superset-ui-encodable/src/encoders/Encoder.ts

+    channelNames
+      .map(name => this.channels[name])
+      .forEach(c => {
+        if (isNotArray(c) && c.hasLegend() && isFieldDef(c.definition)) {


do we not care about channels encoded as arrays in legends?

At this point, the channels that are array are fields for tooltip. I tried to think if we want to support other channels that require legends such as color as array of multiple fields but that makes thing very complicated. Better create a derived field field1,field2 in the dataset and use that derived field instead. (vega also has support for formula instead of field for this kind of purpose, which I haven't explored how to port)

williaster · 2019-10-03T16:58:06Z

packages/superset-ui-encodable/src/encoders/ChannelEncoder.ts

@@ -126,4 +126,8 @@ export default class ChannelEncoder<Def extends ChannelDef<Output>, Output exten
  isY() {
    return isY(this.channelType);
  }
+
+  hasLegend() {
+    return this.definition.legend !== false;


since it looks like legend is optional, should this check if it's set to true?

EDIT: I see that the complete encoding specifies false or { ... }.

williaster · 2019-10-04T03:18:30Z

packages/superset-ui-encodable/src/encoders/createEncoderFactory.ts

+import { EncodingConfig, DeriveChannelTypes, DeriveEncoding } from '../types/Encoding';
+import mergeEncoding from '../utils/mergeEncoding';
+
+type CreateEncoderFactoryParams<Config extends EncodingConfig> = {


this type is beautiful 😻

williaster · 2019-10-04T03:28:20Z

packages/superset-ui-encodable/src/types/Encoding.ts

+};
+
+export type DeriveChannelTypes<Config extends EncodingConfig> = {
+  readonly [k in keyof Config]: Config[k]['0'];


#TIL, did not know that you could index types like this.

interesting it has to be a string and not a number.

williaster · 2019-10-04T03:30:22Z

packages/superset-ui-encodable/src/types/Encoding.ts

+
+export type DeriveChannelEncoders<Config extends EncodingConfig> = {
+  readonly [k in keyof Config]: Config[k]['2'] extends 'multiple'
+    ? ChannelEncoder<ChannelTypeToDefMap<Config[k]['1']>[Config[k]['0']]>[]


so advanced! 🤯

This is like 3rd-4th time I rewrote the whole thing lol.

kristw requested a review from a team as a code owner October 2, 2019 08:04

pull-request-size bot added the size/L label Oct 2, 2019

kristw added 7 commits October 2, 2019 02:38

feat: add encoder

5cbf169

feat: add encoder

05cb13e

refactor: revamp encoding types and derivations

029aa89

test: add unit tests

1cb2716

fix: unit tests

9cf70d5

test: add unit tests

623f036

fix: remove unused code

9ef9389

kristw force-pushed the kristw--encoder branch from 6ea548e to 9ef9389 Compare October 2, 2019 09:39

kristw changed the title ~~[WIP] feat: Add Encoder~~ feat: Add Encoder Oct 2, 2019

williaster approved these changes Oct 4, 2019

View reviewed changes

fix: channeltype

34960fd

kristw merged commit 03aa6b3 into master Oct 4, 2019

delete-merged-branch bot deleted the kristw--encoder branch October 4, 2019 22:16

kristw pushed a commit that referenced this pull request Apr 17, 2020

feat: add Korea(South) to Country Maps (#230)

7ca4b8d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add Encoder #230

feat: Add Encoder #230

kristw commented Oct 2, 2019 •

edited

Loading

netlify bot commented Oct 2, 2019 •

edited

Loading

codecov bot commented Oct 2, 2019 •

edited

Loading

williaster left a comment

williaster Oct 3, 2019

williaster Oct 3, 2019

kristw Oct 4, 2019

williaster Oct 3, 2019

williaster Oct 4, 2019

williaster Oct 4, 2019

williaster Oct 4, 2019

kristw Oct 4, 2019

feat: Add Encoder #230

feat: Add Encoder #230

Conversation

kristw commented Oct 2, 2019 • edited Loading

Example Usage

netlify bot commented Oct 2, 2019 • edited Loading

codecov bot commented Oct 2, 2019 • edited Loading

Codecov Report

williaster left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kristw commented Oct 2, 2019 •

edited

Loading

netlify bot commented Oct 2, 2019 •

edited

Loading

codecov bot commented Oct 2, 2019 •

edited

Loading