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

feat: convert dataset to table class #1239

Merged
merged 14 commits into from
Feb 2, 2021
2 changes: 1 addition & 1 deletion docs/RFC/table-class.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export type KeplerField = {
id: string;
name: string;
format: string;
tableFieldIndex: numberstring;
fieldIdx: number;
type: string;

// meta data, storing domain and mappedValues
Expand Down
6 changes: 3 additions & 3 deletions src/actions/actions.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ import {UiState} from 'reducers/ui-state-updaters';
* Input dataest parsed to addDataToMap
*/
export type ProtoDataset = {
info?: {
id: string;
label: string;
info: {
id?: string;
label?: string;
format?: string;
color?: RGBColor;
};
Expand Down
2 changes: 1 addition & 1 deletion src/components/common/field-selector.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ const FieldType = PropTypes.oneOfType([
format: PropTypes.string,
id: PropTypes.string,
name: PropTypes.string,
tableFieldIndex: PropTypes.number,
fieldIdx: PropTypes.number,
type: PropTypes.number
})
]);
Expand Down
4 changes: 1 addition & 3 deletions src/components/common/item-selector/item-selector.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,9 +313,7 @@ class ItemSelector extends Component {
light={this.props.inputTheme === 'light'}
/>
) : (
<FormattedMessage
id={this.props.placeholder || 'placeholder.selectValue'}
/>
<FormattedMessage id={this.props.placeholder || 'placeholder.selectValue'} />
)}
</DropdownSelectValue>
{this.props.erasable && hasValue ? (
Expand Down
10 changes: 2 additions & 8 deletions src/components/common/range-slider.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,20 +126,14 @@ export default function RangeSliderFactory(RangePlot) {
_setRangeVal1 = val => {
const {value0, range, onChange} = this.props;
const val1 = Number(val);
onChange([
value0,
clamp([value0, range[1]], this._roundValToStep(val1))
]);
onChange([value0, clamp([value0, range[1]], this._roundValToStep(val1))]);
return true;
};

_setRangeVal0 = val => {
const {value1, range, onChange} = this.props;
const val0 = Number(val);
onChange([
clamp([range[0], value1], this._roundValToStep(val0)),
value1
]);
onChange([clamp([range[0], value1], this._roundValToStep(val0)), value1]);
return true;
};

Expand Down
2 changes: 1 addition & 1 deletion src/components/common/slider/slider.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import styled from 'styled-components';
import SliderHandle from './slider-handle';
import SliderBarHandle from './slider-bar-handle';
import {normalizeSliderValue} from 'utils/data-utils';
import { clamp } from 'utils/data-utils';
import {clamp} from 'utils/data-utils';

function noop() {}

Expand Down
10 changes: 6 additions & 4 deletions src/components/map-container.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import EditorFactory from './editor/editor';

// utils
import {generateMapboxLayers, updateMapboxLayers} from 'layers/mapbox-utils';
import {OVERLAY_TYPE} from 'layers/base-layer';
import {setLayerBlending} from 'utils/gl-utils';
import {transformRequest} from 'utils/map-style-utils/mapbox-utils';
import {getLayerHoverProp, renderDeckGlLayer} from 'utils/layer-utils';
Expand All @@ -48,6 +47,7 @@ import {
GEOCODER_LAYER_ID,
THROTTLE_NOTIFICATION_TIME
} from 'constants/default-settings';
import {OVERLAY_TYPE} from 'layers/base-layer';

/** @type {{[key: string]: React.CSSProperties}} */
const MAP_STYLE = {
Expand Down Expand Up @@ -484,7 +484,8 @@ export default function MapContainerFactory(MapPopover, MapControl, Editor) {
visStateActions,
interactionConfig,
editor,
index
index,
isExport
} = this.props;

const layersToRender = this.layersToRenderSelector(this.props);
Expand All @@ -503,7 +504,8 @@ export default function MapContainerFactory(MapPopover, MapControl, Editor) {
transformRequest
};

const isEdit = mapControls.mapDraw ? mapControls.mapDraw.active : false;
const isEdit = (mapControls.mapDraw || {}).active;

const hasGeocoderLayer = layers.find(l => l.id === GEOCODER_LAYER_ID);

return (
Expand All @@ -512,7 +514,7 @@ export default function MapContainerFactory(MapPopover, MapControl, Editor) {
datasets={datasets}
dragRotate={mapState.dragRotate}
isSplit={Boolean(mapLayers)}
isExport={this.props.isExport}
isExport={isExport}
layers={layers}
layersToRender={layersToRender}
mapIndex={index}
Expand Down
2 changes: 1 addition & 1 deletion src/layers/aggregation-layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export const getValueAggrFunc = (field, aggregation) => {
return points => {
return field
? aggregate(
points.map(p => p.data[field.tableFieldIndex - 1]),
points.map(p => field.valueAccessor(p.data)),
aggregation
)
: points.length;
Expand Down
84 changes: 25 additions & 59 deletions src/layers/base-layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,22 +48,12 @@ import {

import {generateHashId, isPlainObject} from 'utils/utils';

import {
getSampleData,
getLatLngBounds,
maybeToDate,
getSortingFunction,
notNullorUndefined
} from 'utils/data-utils';
import {getSampleData, getLatLngBounds, notNullorUndefined} from 'utils/data-utils';

import {
getQuantileDomain,
getOrdinalDomain,
getLogDomain,
getLinearDomain
} from 'utils/data-scale-utils';
import {hexToRgb, getColorGroupByName, reverseColorRange} from 'utils/color-utils';

/** @typedef {import('./index').Layer} LayerClass} */

/**
* Approx. number of points to sample in a large data set
* @type {number}
Expand Down Expand Up @@ -91,9 +81,10 @@ function* generateColor() {
}

export const colorMaker = generateColor();
const defaultGetFieldValue = (field, d) => d[field.tableFieldIndex - 1];
const defaultGetFieldValue = (field, d) => field.valueAccessor(d);

export default class Layer {
/** @type {LayerClass} */
class Layer {
constructor(props = {}) {
this.id = props.id || generateHashId(6);

Expand All @@ -103,6 +94,7 @@ export default class Layer {
// visConfigSettings
this.visConfigSettings = {};

// @ts-ignore
this.config = this.getDefaultLayerConfig({
columns: this.getLayerColumns(),
...props
Expand Down Expand Up @@ -240,7 +232,7 @@ export default class Layer {
prev[key] = requiredFields.length
? requiredFields.map(f => ({
value: f.name,
fieldIdx: f.tableFieldIndex - 1
fieldIdx: f.fieldIdx
}))
: null;
return prev;
Expand All @@ -259,7 +251,7 @@ export default class Layer {
// combinations, e. g. if column a has 2 matched, column b has 3 matched
// 6 possible column pairs will be returned
const allKeys = Object.keys(requiredColumns);
const pointers = allKeys.map((k, i) => (i === allKeys.length - 1 ? -1 : 0));
const pointers = allKeys.map((k, i) => ((i === allKeys.length - 1 ? -1 : 0)));
const countPerKey = allKeys.map(k => requiredColumns[k].length);
const pairs = [];

Expand Down Expand Up @@ -357,7 +349,7 @@ export default class Layer {
const update = field
? {
value: field.name,
fieldIdx: field.tableFieldIndex - 1
fieldIdx: field.fieldIdx
}
: {value: null, fieldIdx: -1};

Expand Down Expand Up @@ -682,10 +674,10 @@ export default class Layer {
hasAllColumns() {
const {columns} = this.config;
return (
columns &&
(columns &&
Object.values(columns).every(v => {
return Boolean(v.optional || (v.value && v.fieldIdx > -1));
})
}))
);
}

Expand All @@ -708,11 +700,11 @@ export default class Layer {

shouldRenderLayer(data) {
return (
this.type &&
(this.type &&
this.config.isVisible &&
this.hasAllColumns() &&
this.hasLayerData(data) &&
typeof this.renderLayer === 'function'
typeof this.renderLayer === 'function')
);
}

Expand Down Expand Up @@ -869,6 +861,9 @@ export default class Layer {
}

updateData(datasets, oldLayerData) {
if (!this.config.dataId) {
return {};
}
const layerDataset = datasets[this.config.dataId];
const {allData} = datasets[this.config.dataId];

Expand Down Expand Up @@ -900,8 +895,8 @@ export default class Layer {
* @returns {object} layer
*/
updateLayerDomain(datasets, newFilter) {
const dataset = this.getDataset(datasets);
if (!dataset) {
const table = this.getDataset(datasets);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interchangeable table/dataset variables are quite confusing. Do we plan to unify this in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea i agree, let's just keep calling it dataset in kepler.gl, because it is used everywhere

if (!table) {
return this;
}
Object.values(this.visualChannels).forEach(channel => {
Expand All @@ -911,7 +906,7 @@ export default class Layer {
// no need to update ordinal domain
if (!newFilter || scaleType !== SCALE_TYPES.ordinal) {
const {domain} = channel;
const updatedDomain = this.calculateLayerDomain(dataset, channel);
const updatedDomain = this.calculateLayerDomain(table, channel);
this.updateLayerConfig({[domain]: updatedDomain});
}
});
Expand All @@ -920,7 +915,7 @@ export default class Layer {
}

getDataset(datasets) {
return datasets[this.config.dataId];
return this.config.dataId ? datasets[this.config.dataId] : null;
}

/**
Expand Down Expand Up @@ -1011,7 +1006,6 @@ export default class Layer {
}

calculateLayerDomain(dataset, visualChannel) {
const {allData, filteredIndexForDomain} = dataset;
const {scale} = visualChannel;
const scaleType = this.config[scale];

Expand All @@ -1021,38 +1015,7 @@ export default class Layer {
return defaultDomain;
}

if (!SCALE_TYPES[scaleType]) {
Console.error(`scale type ${scaleType} not supported`);
return defaultDomain;
}

// TODO: refactor to add valueAccessor to field
const fieldIdx = field.tableFieldIndex - 1;
const isTime = field.type === ALL_FIELD_TYPES.timestamp;
const valueAccessor = maybeToDate.bind(null, isTime, fieldIdx, field.format);
const indexValueAccessor = i => valueAccessor(allData[i]);

const sortFunction = getSortingFunction(field.type);

switch (scaleType) {
case SCALE_TYPES.ordinal:
case SCALE_TYPES.point:
// do not recalculate ordinal domain based on filtered data
// don't need to update ordinal domain every time
return getOrdinalDomain(allData, valueAccessor);

case SCALE_TYPES.quantile:
return getQuantileDomain(filteredIndexForDomain, indexValueAccessor, sortFunction);

case SCALE_TYPES.log:
return getLogDomain(filteredIndexForDomain, indexValueAccessor);

case SCALE_TYPES.quantize:
case SCALE_TYPES.linear:
case SCALE_TYPES.sqrt:
default:
return getLinearDomain(filteredIndexForDomain, indexValueAccessor);
}
return dataset.getColumnLayerDomain(field, scaleType) || defaultDomain;
}

hasHoveredObject(objectInfo) {
Expand All @@ -1074,6 +1037,7 @@ export default class Layer {
const fixed = fixedRadius === undefined ? this.config.visConfig.fixedRadius : fixedRadius;
const {radius} = this.config.visConfig;

// @ts-ignore
return fixed ? 1 : (this.config[field] ? 1 : radius) * this.getZoomFactor(mapState);
}

Expand Down Expand Up @@ -1180,3 +1144,5 @@ export default class Layer {
return () => null;
}
}

export default Layer;
2 changes: 1 addition & 1 deletion src/layers/icon-layer/icon-layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ export default class IconLayer extends Layer {
lng: ptPair.pair.lng,
icon: {
value: iconField.name,
fieldIdx: iconField.tableFieldIndex - 1
fieldIdx: iconField.fieldIdx
}
},
isVisible: true
Expand Down
23 changes: 10 additions & 13 deletions src/layers/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
import {RGBColor, RGBAColor} from '../reducers/types';
import {Dataset, Field, Filter, Datasets} from '../reducers/vis-state-updaters';
import {LayerTextLabel, ColorRange, ColorUI} from './layer-factory';

export type LayerVisConfig = {
opacity: number;
colorRange: ColorRange;
};
import {Field, Datasets, KeplerTable} from '../reducers/vis-state-updaters';
import {LayerTextLabel, ColorRange, ColorUI, LayerVisConfig} from './layer-factory';

export type LayerColumns = {
[key: string]: {value: string | null; fieldIdx: number; optional?: boolean};
Expand Down Expand Up @@ -80,16 +75,16 @@ export class Layer {
hasAllColumns(): boolean;
updateLayerConfig(p: Partial<LayerConfig>): Layer;
updateLayerDomain(datasets: Datasets, filter?: Filter): Layer;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this type def deleted? base-layer still has updateLayerDomain method

Copy link
Collaborator

@macrigiuseppe macrigiuseppe Feb 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure why this was deleted but i will revert it back

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably some rebase conflicts
same for

isLayerHovered(objectInfo: any): boolean;
  hasHoveredObject(objectInfo: any): any | null;
  getHoverData(object: any, allData?: Dataset['allData'], fields?: Dataset['fields']): any;

updateLayerVisualChannel(dataset: Dataset, channel: string): Layer;
updateLayerVisualChannel(dataset: KeplerTable, channel: string): Layer;
shouldCalculateLayerData(props: string[]): boolean;
formatLayerData(datasets: Datasets, oldLayerData?: any);
updateLayerColorUI(prop: string, newConfig: Partial<ColorUI>): Layer;
validateVisualChannel(channel: string): void;
isValidToSave(): boolean;
getVisualChannelDescription(key: string): VisualChannelDescription;
isLayerHovered(objectInfo: any): boolean;
hasHoveredObject(objectInfo: any): any | null;
getHoverData(object: any, allData?: Dataset['allData'], fields?: Dataset['fields']): any;
validateVisualChannel(channel: string);
getVisualChannelDescription(key: string): {label: string, measure: string};

static findDefaultLayerProps(dataset: KeplerTable, foundLayers?: any[]);
// static findDefaultColumnField(defaultFields, allFields)
}

export type LayerClassesType = {
Expand All @@ -108,3 +103,5 @@ export type LayerClassesType = {
s2: Layer;
};
export const LayerClasses: LayerClassesType;

export type OVERLAY_TYPE = {[key: string]: string}
2 changes: 1 addition & 1 deletion src/layers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import {default as S2GeometryLayer} from './s2-geometry-layer/s2-geometry-layer'
import {LAYER_TYPES} from './types';

// base layer
export {default as Layer} from './base-layer';
export {default as Layer, OVERLAY_TYPE, colorMaker} from './base-layer';

// individual layers
export const KeplerGlLayers = {
Expand Down
2 changes: 1 addition & 1 deletion src/layers/layer-text-label.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export function getTextOffsetByRadius(radiusScale, getRadius, mapState) {
}

export const textLabelAccessor = textLabel => d => {
const val = d.data[textLabel.field.tableFieldIndex - 1];
const val = textLabel.field.valueAccessor(d.data);
return notNullorUndefined(val) ? String(val) : '';
};

Expand Down
Loading