You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "Boreto1213 (via GitHub)" <gi...@apache.org> on 2026/02/17 10:33:32 UTC
[PR] Feat/h3 hexagon layer [superset]
Boreto1213 opened a new pull request, #38028:
URL: https://github.com/apache/superset/pull/38028
### SUMMARY
Implements H3 Hexagon Layer visualization using DeckGL's H3HexagonLayer component, addressing the community request in [#18643](https://github.com/apache/superset/discussions/18643).
**Closes:** [#18643](https://github.com/apache/superset/discussions/18643)
This implementation provides:
- Interactive H3 hexagon visualization with WebGL rendering
- Support for metric aggregation and color mapping
- Configurable elevation for 3D visualization
- Full integration with Superset's query and filter system
- Zero new dependencies (uses existing DeckGL library)
### SCREENSHOTS OR ANIMATED GIF
**Chart type selector:**
<img width="2560" height="1294" alt="screencapture-localhost-8088-chart-add-2026-02-02-09_54_20" src="https://github.com/user-attachments/assets/94852fd8-a18f-4d3d-a940-43263da48fec" />
*H3 Hexagon Layer now available as visualization type*
**Control panel:**
<img width="2560" height="1976" alt="screencapture-localhost-8088-explore-2026-02-02-09_56_11" src="https://github.com/user-attachments/assets/541be1af-7fef-4443-8e1a-5b22243a88bb" />
*Configuration options for H3 visualization*
**Rendered visualization:**
<img width="2559" height="1293" alt="Screenshot 2026-02-02 at 09 59 36" src="https://github.com/user-attachments/assets/0fc09430-40c2-4662-bd5f-e49169a79490" />
*Interactive H3 hexagon visualization with tooltips, zoom, and pan*
**Dashboard integration:**
<img width="2560" height="1294" alt="screencapture-localhost-8088-superset-dashboard-12-2026-02-02-09_49_40" src="https://github.com/user-attachments/assets/73a34a63-cd8f-442d-9816-bfa87319d73b" />
*H3 layer integrated in dashboard with filters*
### TESTING INSTRUCTIONS
#### Test Data Preparation
**Sample data for testing**
```sql
-- Simple test dataset
SELECT * FROM (
VALUES
('882a1072c3fffff', 100, 'TypeA'),
('882a1072c7fffff', 250, 'TypeB'),
('882a10728bfffff', 175, 'TypeA'),
('882a10728ffffff', 320, 'TypeC')
) AS t(h3_index, metric_value, category);
```
#### Verification Steps
**1. Create H3 Hexagon Layer chart:**
- Navigate to **Charts** → Click **+ Chart** button
- Select a dataset containing H3 index column
- In visualization type selector, choose **H3 Hexagon Layer**
- Click **Create new chart**
**2. Configure the chart:**
*Query section:*
- **H3 index column:** Select column with H3 hexagon indices
- **Metric:** Choose aggregation metric (e.g., COUNT(\*), SUM(value))
- **Filters:** (optional) Add any filters
*Display section:*
- **Color scheme:** Choose color palette for metric visualization
- **Elevation scale:** Adjust multiplier for 3D height (0-100)
- **Opacity:** Set layer transparency (0-1)
- **Coverage:** Hexagon coverage of H3 cell (0-1)
**3. Verify core functionality:**
- [ ] Chart renders without console errors
- [ ] Hexagons display correctly at initial viewport
- [ ] Tooltips appear on hexagon hover showing:
- H3 index
- Metric value
- Any additional dimensions
- [ ] Color scale applies correctly based on metric
### ADDITIONAL INFORMATION
- [x] Has associated issue: #18643
- [ ] Required feature flags: None
- [ ] API changes: None
- [ ] DB migration required: No
- [ ] Breaking changes: No
**Dependencies:**
- ✅ Uses existing DeckGL library (v8.x) already in Superset
- ✅ H3HexagonLayer component available in current DeckGL version
- ✅ No new npm packages required
- ✅ No new Python packages required
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(chart): h3 hexagon layer [superset]
Posted by "Boreto1213 (via GitHub)" <gi...@apache.org>.
Boreto1213 commented on PR #38028:
URL: https://github.com/apache/superset/pull/38028#issuecomment-4978664599
> Hi @rusackas, @Boreto1213 - following up here. If it's helpful, I'm happy to pick up the rebase so this doesn't stay stuck
Hi @DamianPendrak, yeah you can go ahead with the rebase. Thanks for the help. :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(chart): h3 hexagon layer [superset]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #38028:
URL: https://github.com/apache/superset/pull/38028#discussion_r2874011717
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/package.json:
##########
@@ -52,7 +52,8 @@
"prop-types": "^15.8.1",
"underscore": "^1.13.7",
"urijs": "^1.19.11",
- "xss": "^1.0.15"
+ "xss": "^1.0.15",
+ "h3-js": "^4.4.0"
Review Comment:
The PR description claims "Zero new dependencies", but this change adds a new direct dependency on `h3-js`. Either update the PR description to reflect the added dependency, or refactor to avoid introducing it (e.g., reuse an existing dependency/version already present in the workspace).
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/H3Hexagon/buildQuery.ts:
##########
@@ -0,0 +1,88 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+ buildQueryContext,
+ ensureIsArray,
+ normalizeOrderBy,
+ QueryFormColumn,
+ QueryFormData,
+} from '@superset-ui/core';
+import { addTooltipColumnsToQuery } from '../buildQueryUtils';
+
+export interface H3FormData extends QueryFormData {
+ h3_index: string | string[];
+ metric?: string;
+ js_columns?: string[];
+ tooltip_contents?: unknown[];
+}
+
+export default function buildQuery(formData: H3FormData) {
+ let { h3_index: h3Index } = formData;
+ const {
+ metric,
+ js_columns: jsColumns,
+ tooltip_contents: tooltipContents,
+ } = formData;
+
+ if (Array.isArray(h3Index)) {
+ h3Index = h3Index[0];
+ }
+
+ if (!h3Index) {
+ throw new Error('H3 index is required');
+ }
+
+ return buildQueryContext(formData, {
+ buildQuery: baseQueryObject => {
+ let columns: QueryFormColumn[] = [h3Index];
+ const metrics = metric ? [metric] : [];
+
+ if (jsColumns?.length) {
+ jsColumns.forEach((col: string) => {
+ if (!columns.includes(col)) {
+ columns.push(col);
+ }
+ });
+ }
+
+ columns = addTooltipColumnsToQuery(columns, tooltipContents);
+
+ const filters = ensureIsArray(baseQueryObject.filters || []);
+ filters.push({
+ col: h3Index,
+ op: 'IS NOT NULL',
+ });
Review Comment:
This filter clause uses `col: h3Index`, but `h3Index` may be a column object (from the DnD column control). Filter clauses expect a string column name, so this can produce an invalid query payload. Use `getColumnLabel(h3Index)` (or equivalent) when constructing `filters`.
##########
superset-frontend/package-lock.json:
##########
@@ -53769,6 +53804,17 @@
"internmap": "^1.0.0"
}
},
+ "plugins/legacy-preset-chart-deckgl/node_modules/h3-js": {
+ "version": "4.4.0",
+ "resolved": "https://registry.npmjs.org/h3-js/-/h3-js-4.4.0.tgz",
+ "integrity": "sha512-DvJh07MhGgY2KcC4OeZc8SSyA+ZXpdvoh6uCzGpoKvWtZxJB+g6VXXC1+eWYkaMIsLz7J/ErhOalHCpcs1KYog==",
+ "license": "Apache-2.0",
Review Comment:
`package-lock.json` now installs `h3-js@4.4.0` under `plugins/legacy-preset-chart-deckgl` while another `h3-js` version is already present at the workspace root. Multiple versions increase bundle size and can cause subtle mismatches. Consider aligning to a single version so this dependency can be deduped.
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/H3Hexagon/buildQuery.ts:
##########
@@ -0,0 +1,88 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+ buildQueryContext,
+ ensureIsArray,
+ normalizeOrderBy,
+ QueryFormColumn,
+ QueryFormData,
+} from '@superset-ui/core';
+import { addTooltipColumnsToQuery } from '../buildQueryUtils';
+
+export interface H3FormData extends QueryFormData {
+ h3_index: string | string[];
+ metric?: string;
+ js_columns?: string[];
+ tooltip_contents?: unknown[];
+}
+
+export default function buildQuery(formData: H3FormData) {
+ let { h3_index: h3Index } = formData;
+ const {
+ metric,
+ js_columns: jsColumns,
Review Comment:
This new layer adds custom query-building/transform logic but does not add unit tests. Other DeckGL layers in this preset include `buildQuery.test.ts` coverage; adding similar tests for `buildQuery`/`transformProps` (H3 column selection, metric handling, invalid H3 values) would help prevent regressions.
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/H3Hexagon/transformProps.ts:
##########
@@ -0,0 +1,95 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { ChartProps, getMetricLabel } from '@superset-ui/core';
+import {
+ createBaseTransformResult,
+ getRecordsFromQuery,
+ parseMetricValue,
+} from '../transformUtils';
+
+interface H3Feature {
+ hexagon: string;
+ fillColor?: number[];
+ elevation?: number;
+ [key: string]: unknown;
+}
+
+export default function transformProps(chartProps: ChartProps) {
+ const { formData } = chartProps;
+ const { h3_index: h3IndexRaw, metric, js_columns: jsColumns } = formData;
+
+ const records = getRecordsFromQuery(chartProps.queriesData);
+
+ // Resolve the H3 column name
+ const h3Index = (() => {
+ const columnName = Array.isArray(h3IndexRaw) ? h3IndexRaw[0] : h3IndexRaw;
+ if (columnName) {
+ return columnName;
+ }
Review Comment:
`h3_index` is configured via `sharedControls.groupby`, which commonly yields a column object (or array of column objects), not a plain string. Returning that directly from `h3Index` means `record[h3Index]` will coerce the key to "[object Object]" and produce empty/invalid hexagon values. Convert the selected column to a string label (e.g., `getColumnLabel`) before using it to index query records.
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/H3Hexagon/H3Hexagon.tsx:
##########
@@ -0,0 +1,109 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { H3HexagonLayer } from '@deck.gl/geo-layers';
+import { cellToBoundary } from 'h3-js';
+import { JsonObject, QueryFormData, t } from '@superset-ui/core';
+import { commonLayerProps } from '../common';
+import { createDeckGLComponent, GetLayerType } from '../../factory';
+import TooltipRow from '../../TooltipRow';
+import { createTooltipContent } from '../../utilities/tooltipUtils';
+import sandboxedEval from '../../utils/sandbox';
+
+function defaultTooltipContent(_formData: QueryFormData) {
+ const TooltipContent = (o: JsonObject) => {
+ const obj = o.object as any;
+ return (
+ <div className="deckgl-tooltip">
+ <TooltipRow label={`${t('H3 Index')}: `} value={obj?.hexagon} />
+ {obj?.elevation !== undefined && (
+ <TooltipRow label={`${t('Value')}: `} value={`${obj.elevation}`} />
+ )}
+ </div>
+ );
+ };
+ TooltipContent.displayName = 'H3HexagonTooltipContent';
+ return TooltipContent;
+}
+
+export function getPoints(data: JsonObject[]) {
+ const points: [number, number][] = [];
+
+ data.forEach((d: JsonObject) => {
+ const boundary = cellToBoundary(d.hexagon);
+ if (boundary && boundary.length > 0) {
+ const point: [number, number] = [boundary[0][1], boundary[0][0]];
+ points.push(point);
+ }
+ });
+
+ return points;
+}
+
+export const getLayer: GetLayerType<H3HexagonLayer> = function ({
+ formData,
+ payload,
+ setTooltip,
+ setDataMask,
+ filterState,
+ onContextMenu,
+ emitCrossFilters,
+}) {
+ const fd = formData;
+ const {
+ extruded = true,
+ coverage = 1,
+ elevation_scale: elevationScale = 1,
+ } = fd;
+
+ let data = [...payload.data.features];
+
+ if (fd.js_data_mutator) {
+ const jsFnMutator = sandboxedEval(fd.js_data_mutator);
+ data = jsFnMutator(data);
+ }
+
+ const tooltipContentGenerator = createTooltipContent(
+ fd,
+ defaultTooltipContent(fd),
+ );
+
+ return new H3HexagonLayer({
+ id: `h3-hexagon-layer-${fd.slice_id}`,
+ data,
+ extruded: Boolean(extruded),
+ coverage,
+ elevationScale: elevationScale,
+
+ getHexagon: (d: JsonObject) => d.hexagon,
+ getFillColor: (d: JsonObject) => [255, (1 - d.elevation / 500) * 255, 0],
+ getElevation: (d: JsonObject) => d.elevation || 0,
Review Comment:
`getFillColor` uses `d.elevation` without a default, so when no metric is selected (or a record has null), `(1 - d.elevation / 500)` becomes `NaN` and produces an invalid color array. It also ignores the `fill_color_picker` control and hard-codes the `500` scaling. Default elevation to 0 (or compute a domain from data) and incorporate the configured color control(s) so the UI settings affect rendering.
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/H3Hexagon/H3Hexagon.tsx:
##########
@@ -0,0 +1,109 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { H3HexagonLayer } from '@deck.gl/geo-layers';
+import { cellToBoundary } from 'h3-js';
+import { JsonObject, QueryFormData, t } from '@superset-ui/core';
+import { commonLayerProps } from '../common';
+import { createDeckGLComponent, GetLayerType } from '../../factory';
+import TooltipRow from '../../TooltipRow';
+import { createTooltipContent } from '../../utilities/tooltipUtils';
+import sandboxedEval from '../../utils/sandbox';
+
+function defaultTooltipContent(_formData: QueryFormData) {
+ const TooltipContent = (o: JsonObject) => {
+ const obj = o.object as any;
+ return (
+ <div className="deckgl-tooltip">
+ <TooltipRow label={`${t('H3 Index')}: `} value={obj?.hexagon} />
+ {obj?.elevation !== undefined && (
+ <TooltipRow label={`${t('Value')}: `} value={`${obj.elevation}`} />
+ )}
+ </div>
+ );
+ };
+ TooltipContent.displayName = 'H3HexagonTooltipContent';
+ return TooltipContent;
+}
+
+export function getPoints(data: JsonObject[]) {
+ const points: [number, number][] = [];
+
+ data.forEach((d: JsonObject) => {
+ const boundary = cellToBoundary(d.hexagon);
+ if (boundary && boundary.length > 0) {
+ const point: [number, number] = [boundary[0][1], boundary[0][0]];
+ points.push(point);
+ }
Review Comment:
`cellToBoundary(d.hexagon)` will throw for invalid/empty H3 indices. Since data can contain bad values (and transformProps can emit an empty string), this can crash autozoom/rendering for the entire chart. Guard with validation/try-catch and skip invalid cells when building points.
```suggestion
const hexagon = d.hexagon;
if (!hexagon) {
// Skip entries without a valid H3 index
return;
}
try {
const boundary = cellToBoundary(hexagon);
if (boundary && boundary.length > 0) {
const point: [number, number] = [boundary[0][1], boundary[0][0]];
points.push(point);
}
} catch {
// Skip entries with invalid H3 indices that cause cellToBoundary to throw
}
```
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/H3Hexagon/H3Hexagon.tsx:
##########
@@ -0,0 +1,109 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { H3HexagonLayer } from '@deck.gl/geo-layers';
+import { cellToBoundary } from 'h3-js';
+import { JsonObject, QueryFormData, t } from '@superset-ui/core';
+import { commonLayerProps } from '../common';
+import { createDeckGLComponent, GetLayerType } from '../../factory';
+import TooltipRow from '../../TooltipRow';
+import { createTooltipContent } from '../../utilities/tooltipUtils';
+import sandboxedEval from '../../utils/sandbox';
+
+function defaultTooltipContent(_formData: QueryFormData) {
+ const TooltipContent = (o: JsonObject) => {
+ const obj = o.object as any;
+ return (
Review Comment:
Avoid using `any` here (the repo explicitly disallows `any` in new TS). `o.object` can be typed based on the layer datum shape (e.g., `{ hexagon: string; elevation?: number; ... }`) and then safely accessed without dropping type safety.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(chart): h3 hexagon layer [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #38028:
URL: https://github.com/apache/superset/pull/38028#discussion_r3685923626
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/H3Hexagon/transformProps.ts:
##########
@@ -0,0 +1,102 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import {
+ ChartProps,
+ getColumnLabel,
+ getMetricLabel,
+ QueryFormColumn,
+} from '@superset-ui/core';
+import {
+ createBaseTransformResult,
+ getRecordsFromQuery,
+ parseMetricValue,
+} from '../transformUtils';
+
+export interface H3Feature {
+ hexagon: string;
+ fillColor?: number[];
+ elevation?: number;
+ [key: string]: unknown;
+}
+
+export default function transformProps(chartProps: ChartProps) {
+ const { formData } = chartProps;
+ const { h3_index: h3IndexRaw, metric, js_columns: jsColumns } = formData;
+
+ const records = getRecordsFromQuery(chartProps.queriesData);
+
+ // Resolve the H3 column label, deriving a string consistently
+ const h3Index = ((): string | undefined => {
+ const column: QueryFormColumn | undefined = Array.isArray(h3IndexRaw)
+ ? h3IndexRaw[0]
+ : h3IndexRaw;
+ if (column) {
+ return getColumnLabel(column);
+ }
+ // Auto-detect H3 column if not specified
+ if (records.length > 0) {
+ const firstRecord = records[0];
+ const possibleH3Keys = Object.keys(firstRecord).filter(
+ (key: string) =>
+ key.toLowerCase().includes('h3') || key.toLowerCase().includes('hex'),
+ );
+ if (possibleH3Keys.length > 0) {
+ return possibleH3Keys[0];
+ }
+ }
+ return undefined;
+ })();
+
+ const metricLabel = metric ? getMetricLabel(metric) : undefined;
+
+ const features: H3Feature[] = records.map((record, index) => {
+ const hexagonValue = h3Index ? record[h3Index] : undefined;
+ const feature: H3Feature = {
+ hexagon: String(hexagonValue || ''),
+ };
+
+ if (metricLabel && record[metricLabel] != null) {
+ const value = parseMetricValue(record[metricLabel]);
+ if (value !== undefined) {
+ feature.elevation = value;
+ }
+ }
+
+ if (jsColumns?.length) {
+ jsColumns.forEach((col: string) => {
+ if (record[col] !== undefined) {
+ feature[col] = record[col];
+ }
+ });
+ }
+
+ Object.keys(record).forEach(key => {
+ if (key !== h3Index && key !== metricLabel) {
+ feature[key] = record[key];
Review Comment:
A returned tooltip/JS column named `hexagon` or `elevation` overwrites the H3 index or parsed metric derived above, so the layer can render an invalid cell or use the wrong height/color. Could we exclude those reserved keys when copying record fields?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(chart): h3 hexagon layer [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38028:
URL: https://github.com/apache/superset/pull/38028#issuecomment-3986085559
<!-- Bito Reply -->
Yes, the dependencies in package.json can be sorted alphabetically for consistency. Here's a suggested sorted version of the dependencies section from the diff.
**superset-frontend/plugins/legacy-preset-chart-deckgl/package.json**
```
"h3-js": "^4.4.0",
"prop-types": "^15.8.1",
"underscore": "^1.13.7",
"urijs": "^1.19.11",
"xss": "^1.0.15"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(chart): h3 hexagon layer [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #38028:
URL: https://github.com/apache/superset/pull/38028#discussion_r4151427876
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/H3Hexagon/buildQuery.ts:
##########
@@ -0,0 +1,93 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+ buildQueryContext,
+ ensureIsArray,
+ getColumnLabel,
+ normalizeOrderBy,
+ QueryFormColumn,
+ QueryFormData,
+} from '@superset-ui/core';
+import { addTooltipColumnsToQuery } from '../buildQueryUtils';
+
+export interface H3FormData extends QueryFormData {
+ h3_index: QueryFormColumn | QueryFormColumn[];
+ metric?: string;
+ js_columns?: string[];
+ tooltip_contents?: unknown[];
+}
+
+export default function buildQuery(formData: H3FormData) {
+ const {
+ h3_index: h3IndexRaw,
+ metric,
+ js_columns: jsColumns,
+ tooltip_contents: tooltipContents,
+ } = formData;
+
+ const h3Index: QueryFormColumn | undefined = Array.isArray(h3IndexRaw)
+ ? h3IndexRaw[0]
+ : h3IndexRaw;
+
+ if (!h3Index) {
+ throw new Error('H3 index is required');
+ }
+
+ const h3IndexLabel = getColumnLabel(h3Index);
+
+ return buildQueryContext(formData, {
+ buildQuery: baseQueryObject => {
+ let columns: QueryFormColumn[] = [h3Index];
+ const metrics = metric ? [metric] : [];
+
+ if (jsColumns?.length) {
+ const existingLabels = new Set(columns.map(getColumnLabel));
+ jsColumns.forEach((col: string) => {
+ if (!existingLabels.has(col)) {
+ columns.push(col);
+ existingLabels.add(col);
+ }
+ });
+ }
+
+ columns = addTooltipColumnsToQuery(columns, tooltipContents);
+
+ const filters = ensureIsArray(baseQueryObject.filters || []);
+ filters.push({
+ col: h3IndexLabel,
Review Comment:
For an ad-hoc H3 expression, this label is not a physical dataset column, so the backend rejects the null filter instead of applying it. A NULL bucket can then consume the row limit ahead of valid cells; could we pass the ad-hoc column object here and update the test to assert that payload?
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/H3Hexagon/H3Hexagon.tsx:
##########
@@ -0,0 +1,208 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { H3HexagonLayer } from '@deck.gl/geo-layers';
+import { cellToBoundary } from 'h3-js';
+import { JsonObject, QueryFormData } from '@superset-ui/core';
+import { t } from '@apache-superset/core';
+import { Color } from '@deck.gl/core';
+import { commonLayerProps, getColorForBreakpoints } from '../common';
+import { createDeckGLComponent, GetLayerType } from '../../factory';
+import TooltipRow from '../../TooltipRow';
+import { createTooltipContent } from '../../utilities/tooltipUtils';
+import sandboxedEval from '../../utils/sandbox';
+import {
+ BucketsWithColorScale,
+ getBreakPointColorScaler,
+ TRANSPARENT_COLOR_ARRAY,
+} from '../../utils';
+import { COLOR_SCHEME_TYPES } from '../../utilities/utils';
+import { DEFAULT_DECKGL_COLOR } from '../../utilities/Shared_DeckGL';
+import { H3Feature } from './transformProps';
+
+function defaultTooltipContent(_formData: QueryFormData) {
+ const TooltipContent = (o: JsonObject) => {
+ const obj = o.object as H3Feature | undefined;
+ return (
+ <div className="deckgl-tooltip">
+ <TooltipRow label={`${t('H3 Index')}: `} value={obj?.hexagon ?? ''} />
+ {obj?.elevation !== undefined && (
+ <TooltipRow label={`${t('Value')}: `} value={`${obj.elevation}`} />
+ )}
+ </div>
+ );
+ };
+ TooltipContent.displayName = 'H3HexagonTooltipContent';
+ return TooltipContent;
+}
+
+export function getPoints(data: H3Feature[]) {
+ const points: [number, number][] = [];
+
+ data.forEach(d => {
+ const { hexagon } = d;
+ if (!hexagon) {
+ return;
+ }
+
+ try {
+ const boundary = cellToBoundary(hexagon);
+ if (boundary && boundary.length > 0) {
+ const point: [number, number] = [boundary[0][1], boundary[0][0]];
+ points.push(point);
+ }
+ } catch {
+ // Skip entries with invalid H3 indices that cause cellToBoundary to throw
+ }
+ });
+
+ return points;
+}
+
+export const getLayer: GetLayerType<H3HexagonLayer> = function ({
+ formData,
+ payload,
+ setTooltip,
+ setDataMask,
+ filterState,
+ onContextMenu,
+ emitCrossFilters,
+}) {
+ const fd = formData;
+ const {
+ extruded = true,
+ coverage = 1,
+ elevation_scale: elevationScale = 1,
+ } = fd;
+
+ let data = [...payload.data.features];
+
+ if (fd.js_data_mutator) {
+ const jsFnMutator = sandboxedEval(fd.js_data_mutator);
+ data = jsFnMutator(data);
+ }
+
+ const fc = fd.fill_color_picker as
+ | { r: number; g: number; b: number; a: number }
+ | undefined;
+ const defaultBreakpointColor = fd.default_breakpoint_color as
+ | { r: number; g: number; b: number; a: number }
+ | undefined;
+ const fixedColor: Color = fc
+ ? [fc.r, fc.g, fc.b, 255 * fc.a]
+ : [
+ DEFAULT_DECKGL_COLOR.r,
+ DEFAULT_DECKGL_COLOR.g,
+ DEFAULT_DECKGL_COLOR.b,
+ DEFAULT_DECKGL_COLOR.a * 255,
+ ];
+
+ const colorSchemeType = fd.color_scheme_type;
+ const accessor = (d: JsonObject) =>
+ typeof d?.elevation === 'number' ? d.elevation : undefined;
+
+ let getFillColor: (d: JsonObject) => Color;
+ switch (colorSchemeType) {
+ case COLOR_SCHEME_TYPES.fixed_color: {
+ getFillColor = () => fixedColor;
+ break;
+ }
+ case COLOR_SCHEME_TYPES.linear_palette: {
+ // Without a metric there is nothing to scale by, fall back to the fixed color
+ getFillColor = fd.metric
Review Comment:
With the default linear palette, a valid cell whose metric is 0 becomes transparent because `getBreakPointColorScaler` treats a falsy accessor value as missing. Could we preserve zero-valued cells and cover that case in the layer tests?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(chart): h3 hexagon layer [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #38028:
URL: https://github.com/apache/superset/pull/38028#issuecomment-3913809809
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 340b03b002a34ce9d7547eb8fec5a7f222401a6b |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/699443fdb43aca000889ddc5 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-38028--superset-docs-preview.netlify.app](https://deploy-preview-38028--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-38028--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(chart): h3 hexagon layer [superset]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #38028:
URL: https://github.com/apache/superset/pull/38028#discussion_r2874011787
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/H3Hexagon/buildQuery.ts:
##########
@@ -0,0 +1,88 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+ buildQueryContext,
+ ensureIsArray,
+ normalizeOrderBy,
+ QueryFormColumn,
+ QueryFormData,
+} from '@superset-ui/core';
+import { addTooltipColumnsToQuery } from '../buildQueryUtils';
+
+export interface H3FormData extends QueryFormData {
+ h3_index: string | string[];
Review Comment:
`h3_index` comes from a column-select control and may be a column object/array rather than a string. Typing it as `string | string[]` is misleading and can mask runtime issues elsewhere (e.g., filters and record-key lookups). Update this to `QueryFormColumn | QueryFormColumn[]` (or similar) and consistently derive a string label when needed.
```suggestion
h3_index: QueryFormColumn | QueryFormColumn[];
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(chart): h3 hexagon layer [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #38028:
URL: https://github.com/apache/superset/pull/38028#issuecomment-4770501786
@Boreto1213 nice work — addresses the long-standing #18643 ask and ships with tests. If you can rebase it, that'd be great, and we can re-review. Let me know if you want me to take a crack at resolving the conflicts on this branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(chart): h3 hexagon layer [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #38028:
URL: https://github.com/apache/superset/pull/38028#issuecomment-5924981268
Heya! This one needs more than a normal rebase, `legacy-preset-chart-deckgl` got folded into `preset-chart-deckgl` on `master` back in #38035, so the whole H3Hexagon layer needs to move into that package instead. The good news is it's mostly mechanical: `preset-chart-deckgl/src/layers/Hex/` has the exact same file shape, and every shared helper you're importing (`common.tsx`, `factory.tsx`, `TooltipRow.tsx`, `utilities/tooltipUtils.tsx`, `utilities/Shared_DeckGL.tsx`, `utils.ts`) still lives at the same relative path, so most imports shouldn't need touching. `h3-js` is also already a transitive lockfile dependency close to the version you want, so it should dedupe cleanly once it's a direct dependency of the new package.
One thing that needs removing rather than porting: #42126 stripped the whole JS-eval customization feature (`js_data_mutator`/`js_tooltip`/`js_onclick_href`/`js_columns`) from every deck.gl layer. `H3Hexagon.tsx`, `controlPanel.ts`, `buildQuery.ts` and `transformProps.ts` all still reference it, that'll need to come out along with the "Advanced" control section.
While you're in there, a few of the open threads are still real and worth fixing. The cross-filter click handler throws, since H3 form data doesn't have any of the fields `getCrossFilterDataMask` needs, easiest fix is just not wiring `emitCrossFilters` for this layer, matching CodeAnt's suggested fallback. `getPoints` only samples each hexagon's first boundary vertex for auto-zoom, so edge hexagons get clipped. A tooltip/JS column literally named `hexagon` or `elevation` clobbers the internal field of the same name in `transformProps.ts`. The NULL filter on `h3_index` passes just the column label, so it gets rejected for an ad-hoc expression, needs the raw column object instead. The zero-value-renders-transparent one is actually a pre-existing bug in the shared `getBreakPointColorScaler` in `utils.ts` (`if (!v)` treats a legit `0` metric as missing), not something specific to this layer. DamianPendrak's color-scheme-type selector ask looks already done, just never got marked resol
ved.
Setting this to draft for now given the scope. Holler if you want a hand with any of it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(chart): h3 hexagon layer [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38028:
URL: https://github.com/apache/superset/pull/38028#issuecomment-3915578486
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(chart): h3 hexagon layer [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #38028:
URL: https://github.com/apache/superset/pull/38028#issuecomment-6329363186
@Boreto1213 @DamianPendrak I can't push to this fork, so I'll bring the H3 layer forward on a fresh branch, building on Damian's rebased work and crediting you both.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(chart): h3 hexagon layer [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #38028:
URL: https://github.com/apache/superset/pull/38028#discussion_r3147189358
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/H3Hexagon/H3Hexagon.tsx:
##########
@@ -0,0 +1,208 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { H3HexagonLayer } from '@deck.gl/geo-layers';
+import { cellToBoundary } from 'h3-js';
+import { JsonObject, QueryFormData } from '@superset-ui/core';
+import { t } from '@apache-superset/core';
+import { Color } from '@deck.gl/core';
+import { commonLayerProps, getColorForBreakpoints } from '../common';
+import { createDeckGLComponent, GetLayerType } from '../../factory';
+import TooltipRow from '../../TooltipRow';
+import { createTooltipContent } from '../../utilities/tooltipUtils';
+import sandboxedEval from '../../utils/sandbox';
+import {
+ BucketsWithColorScale,
+ getBreakPointColorScaler,
+ TRANSPARENT_COLOR_ARRAY,
+} from '../../utils';
+import { COLOR_SCHEME_TYPES } from '../../utilities/utils';
+import { DEFAULT_DECKGL_COLOR } from '../../utilities/Shared_DeckGL';
+import { H3Feature } from './transformProps';
+
+function defaultTooltipContent(_formData: QueryFormData) {
+ const TooltipContent = (o: JsonObject) => {
+ const obj = o.object as H3Feature | undefined;
+ return (
+ <div className="deckgl-tooltip">
+ <TooltipRow label={`${t('H3 Index')}: `} value={obj?.hexagon ?? ''} />
+ {obj?.elevation !== undefined && (
+ <TooltipRow label={`${t('Value')}: `} value={`${obj.elevation}`} />
+ )}
+ </div>
+ );
+ };
+ TooltipContent.displayName = 'H3HexagonTooltipContent';
+ return TooltipContent;
+}
+
+export function getPoints(data: H3Feature[]) {
+ const points: [number, number][] = [];
+
+ data.forEach(d => {
+ const { hexagon } = d;
+ if (!hexagon) {
+ return;
+ }
+
+ try {
+ const boundary = cellToBoundary(hexagon);
+ if (boundary && boundary.length > 0) {
+ const point: [number, number] = [boundary[0][1], boundary[0][0]];
+ points.push(point);
Review Comment:
**Suggestion:** Auto-zoom is computed from only the first vertex of each hexagon, not from the full hexagon geometry. This underestimates dataset bounds and can produce a viewport that clips hexagons at the edges. Use all boundary vertices (or the true H3 center for each cell) when building `points` for viewport fitting. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Auto-zoom for `deck_h3_hexagon` may clip edge hexagons.
- ⚠️ Users must pan map despite Auto Zoom enabled.
- ⚠️ H3 Hexagon viewport less accurate than other DeckGL layers.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In Explore, create a new chart with visualization type `deck.gl H3 Hexagon` (`viz_type:
'deck_h3_hexagon'`, plugin wired via `H3HexagonChartPlugin` in
`superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/H3Hexagon/index.ts:50-58`
and registered in the DeckGL preset at `src/layers/preset.ts:44-49`).
2. In the H3 Hexagon control panel, ensure `Auto Zoom` is enabled (it is on by default via
`autozoom` control with `default: true` in
`superset-frontend/plugins/legacy-preset-chart-deckgl/src/utilities/Shared_DeckGL.tsx:139-145`,
and exposed in the H3 panel in the "Map" section as `[autozoom, viewport]` at
`src/layers/H3Hexagon/controlPanel.ts:81-84`).
3. Run a query that returns several H3 cells spread over an area (records are converted to
features with a `hexagon` string in `transformProps` at
`src/layers/H3Hexagon/transformProps.ts:69-77`, producing `payload.data.features` consumed
by the React chart component created via `createDeckGLComponent(getLayer, getPoints)` at
`src/layers/H3Hexagon/H3Hexagon.tsx:208`).
4. When the chart renders, `createDeckGLComponent` computes an auto-zoom viewport by
calling `fitViewport` with `points: getPoints(props.payload.data.features)` in
`src/factory.tsx:98-105`. For H3, `getPoints` (in `H3Hexagon.tsx:54-74`) calls
`cellToBoundary(hexagon)` and, for each feature, only pushes the first boundary vertex
`[boundary[0][1], boundary[0][0]]` into the `points` array (`lines 63-67`). `fitViewport`
then computes bounds from these sample points using `computeBoundsFromPoints(points)`
(`src/utils/fitViewport.ts:41-55`), which ignores all other vertices of each hexagon. For
datasets where the outermost part of the hex geometry lies at a different vertex than
`boundary[0]`, the resulting bounding box underestimates the true spatial extent, so the
fitted viewport can leave parts of edge hexagons just outside the visible area, requiring
manual panning to see the full polygons. This behavior is specific to H3: other DeckGL
layers pass either full polygon coordinates (`Polygon.getPoints` flattens all vertices via
`getPointsFromPolygon` at `src/layers/Polygon/Polygon.tsx:259-260`) or well-defined point
centers (`Hex/Scatter/Grid/Heatmap getPoints` all return `d.position`), so the H3
implementation is more error-prone for auto-zoom.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fplugins%2Flegacy-preset-chart-deckgl%2Fsrc%2Flayers%2FH3Hexagon%2FH3Hexagon.tsx%0A%2A%2ALine%3A%2A%2A%2063%3A67%0A%2A%2AComment%3A%2A%2A%0A%09%2ALogic%20Error%3A%20Auto-zoom%20is%20computed%20from%20only%20the%20first%20vertex%20of%20each%20hexagon%2C%20not%20from%20the%20full%20hexagon%20geometry.%20This%20underestimates%20dataset%20bounds%20and%20can%20produce%20a%20viewport%20that%20clips%20hexagons%20at%20the%20edges.%20Use%20all%20boundary%20vertices%20%28or%20the%20true%20H3%20center%20for%20each%20cell%29%20when%20building%20%60points%60%20for%20viewport%20fitting.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%
2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fplugins%2Flegacy-preset-chart-deckgl%2Fsrc%2Flayers%2FH3Hexagon%2FH3Hexagon.tsx%0A%2A%2ALine%3A%2A%2A%2063%3A67%0A%2A%2AComment%3A%2A%2A%0A%09%2ALogic%20Error%3A%20Auto-zoom%20is%20computed%20from%20only%20the%20first%20vertex%20of%20each%20hexagon%2C%20not%20from%20the%20full%20hexagon%20geometry.%20This%20underestimates%20dataset%20bounds%20and%20can%20produce%20a%20viewport%20that%20clips%20hexagons%20at%20the%20edges.%20Use%20all%20boundary%20vertices%20%28or%20the%20true%20H3%20center%20for%20ea
ch%20cell%29%20when%20building%20%60points%60%20for%20viewport%20fitting.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/H3Hexagon/H3Hexagon.tsx
**Line:** 63:67
**Comment:**
*Logic Error: Auto-zoom is computed from only the first vertex of each hexagon, not from the full hexagon geometry. This underestimates dataset bounds and can produce a viewport that clips hexagons at the edges. Use all boundary vertices (or the true H3 center for each cell) when building `points` for viewport fitting.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38028&comment_hash=87f4527ebdd79cd2fdaff3a1ec89f99005cca84269f9bc4e5bc82926b778f0d6&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38028&comment_hash=87f4527ebdd79cd2fdaff3a1ec89f99005cca84269f9bc4e5bc82926b778f0d6&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(chart): h3 hexagon layer [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #38028:
URL: https://github.com/apache/superset/pull/38028#discussion_r2873972829
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/package.json:
##########
@@ -52,7 +52,8 @@
"prop-types": "^15.8.1",
"underscore": "^1.13.7",
"urijs": "^1.19.11",
- "xss": "^1.0.15"
+ "xss": "^1.0.15",
+ "h3-js": "^4.4.0"
Review Comment:
can we sort this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(chart): h3 hexagon layer [superset]
Posted by "DamianPendrak (via GitHub)" <gi...@apache.org>.
DamianPendrak commented on code in PR #38028:
URL: https://github.com/apache/superset/pull/38028#discussion_r2874625566
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/H3Hexagon/controlPanel.ts:
##########
@@ -0,0 +1,140 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { t, validateNonEmpty } from '@superset-ui/core';
+import {
+ ControlPanelConfig,
+ sections,
+ sharedControls,
+} from '@superset-ui/chart-controls';
+import {
+ jsColumns,
+ jsDataMutator,
+ jsTooltip,
+ jsOnclickHref,
+ mapboxStyle,
+ viewport,
+ autozoom,
+ tooltipContents,
+ tooltipTemplate,
+} from '../../utilities/Shared_DeckGL';
+
+const config: ControlPanelConfig = {
+ controlPanelSections: [
+ sections.legacyTimeseriesTime,
+ {
+ label: t('Query'),
+ expanded: true,
+ controlSetRows: [
+ [
+ {
+ name: 'h3_index',
+ config: {
+ ...sharedControls.groupby,
+ label: t('H3 Column'),
+ description: t('The column containing H3 hexagon indices'),
+ multi: false,
+ validators: [validateNonEmpty],
+ },
+ },
+ ],
+ [
+ {
+ name: 'metric',
+ config: {
+ ...sharedControls.metric,
+ label: t('Metric'),
+ description: t('Metric for hexagon elevation/color'),
+ },
+ },
+ ],
+ ['adhoc_filters'],
+ ['row_limit'],
+ [jsColumns],
+ [tooltipContents],
+ [tooltipTemplate],
+ ],
+ },
+ {
+ label: t('Map'),
+ expanded: true,
+ controlSetRows: [[mapboxStyle], [autozoom, viewport]],
+ },
+ {
+ label: t('Visual'),
+ controlSetRows: [
+ [
+ {
+ name: 'extruded',
+ config: {
+ type: 'CheckboxControl',
+ label: t('Extruded (3D)'),
+ default: true,
+ description: t('Whether to extrude hexagons in 3D'),
+ },
+ },
+ ],
+ [
+ {
+ name: 'coverage',
+ config: {
+ type: 'SliderControl',
+ label: t('Coverage'),
+ default: 1,
+ min: 0,
+ max: 1,
+ step: 0.05,
+ description: t('Hexagon size multiplier (0-1)'),
+ },
+ },
+ ],
+ [
+ {
+ name: 'elevation_scale',
+ config: {
+ type: 'SliderControl',
+ label: t('Elevation Scale'),
+ default: 1,
+ min: 0,
+ max: 100,
+ step: 1,
+ description: t('Multiplier for hexagon elevation'),
+ },
+ },
+ ],
+ [
+ {
+ name: 'fill_color_picker',
+ config: {
+ type: 'ColorPickerControl',
+ label: t('Fill Color'),
+ default: { r: 255, g: 140, b: 0, a: 0.8 },
Review Comment:
There is `PRIMARY_COLOR` that's used in deck.gl layers as the default. There is also color scheme type select for most of the deckgl charts, that allows users to select from fixed color, categorical palette/linear palette, and color breakpoints. Is it possible to implement that here? There are utils like `deckGLCategoricalColorSchemeTypeSelect` and `deckGLFixedColor` in the `Shared_DeckGL.tsx` for color controls. You can check DeckGL Polygon chart as it also uses the `getFillColor` property
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(chart): h3 hexagon layer [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #38028:
URL: https://github.com/apache/superset/pull/38028#discussion_r3147177430
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/H3Hexagon/H3Hexagon.tsx:
##########
@@ -0,0 +1,208 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { H3HexagonLayer } from '@deck.gl/geo-layers';
+import { cellToBoundary } from 'h3-js';
+import { JsonObject, QueryFormData } from '@superset-ui/core';
+import { t } from '@apache-superset/core';
+import { Color } from '@deck.gl/core';
+import { commonLayerProps, getColorForBreakpoints } from '../common';
+import { createDeckGLComponent, GetLayerType } from '../../factory';
+import TooltipRow from '../../TooltipRow';
+import { createTooltipContent } from '../../utilities/tooltipUtils';
+import sandboxedEval from '../../utils/sandbox';
+import {
+ BucketsWithColorScale,
+ getBreakPointColorScaler,
+ TRANSPARENT_COLOR_ARRAY,
+} from '../../utils';
+import { COLOR_SCHEME_TYPES } from '../../utilities/utils';
+import { DEFAULT_DECKGL_COLOR } from '../../utilities/Shared_DeckGL';
+import { H3Feature } from './transformProps';
+
+function defaultTooltipContent(_formData: QueryFormData) {
+ const TooltipContent = (o: JsonObject) => {
+ const obj = o.object as H3Feature | undefined;
+ return (
+ <div className="deckgl-tooltip">
+ <TooltipRow label={`${t('H3 Index')}: `} value={obj?.hexagon ?? ''} />
+ {obj?.elevation !== undefined && (
+ <TooltipRow label={`${t('Value')}: `} value={`${obj.elevation}`} />
+ )}
+ </div>
+ );
+ };
+ TooltipContent.displayName = 'H3HexagonTooltipContent';
+ return TooltipContent;
+}
+
+export function getPoints(data: H3Feature[]) {
+ const points: [number, number][] = [];
+
+ data.forEach(d => {
+ const { hexagon } = d;
+ if (!hexagon) {
+ return;
+ }
+
+ try {
+ const boundary = cellToBoundary(hexagon);
+ if (boundary && boundary.length > 0) {
+ const point: [number, number] = [boundary[0][1], boundary[0][0]];
+ points.push(point);
+ }
+ } catch {
+ // Skip entries with invalid H3 indices that cause cellToBoundary to throw
+ }
+ });
+
+ return points;
+}
+
+export const getLayer: GetLayerType<H3HexagonLayer> = function ({
+ formData,
+ payload,
+ setTooltip,
+ setDataMask,
+ filterState,
+ onContextMenu,
+ emitCrossFilters,
+}) {
+ const fd = formData;
+ const {
+ extruded = true,
+ coverage = 1,
+ elevation_scale: elevationScale = 1,
+ } = fd;
+
+ let data = [...payload.data.features];
+
+ if (fd.js_data_mutator) {
+ const jsFnMutator = sandboxedEval(fd.js_data_mutator);
+ data = jsFnMutator(data);
+ }
+
+ const fc = fd.fill_color_picker as
+ | { r: number; g: number; b: number; a: number }
+ | undefined;
+ const defaultBreakpointColor = fd.default_breakpoint_color as
+ | { r: number; g: number; b: number; a: number }
+ | undefined;
+ const fixedColor: Color = fc
+ ? [fc.r, fc.g, fc.b, 255 * fc.a]
+ : [
+ DEFAULT_DECKGL_COLOR.r,
+ DEFAULT_DECKGL_COLOR.g,
+ DEFAULT_DECKGL_COLOR.b,
+ DEFAULT_DECKGL_COLOR.a * 255,
+ ];
+
+ const colorSchemeType = fd.color_scheme_type;
+ const accessor = (d: JsonObject) =>
+ typeof d?.elevation === 'number' ? d.elevation : undefined;
+
+ let getFillColor: (d: JsonObject) => Color;
+ switch (colorSchemeType) {
+ case COLOR_SCHEME_TYPES.fixed_color: {
+ getFillColor = () => fixedColor;
+ break;
+ }
+ case COLOR_SCHEME_TYPES.linear_palette: {
+ // Without a metric there is nothing to scale by, fall back to the fixed color
+ getFillColor = fd.metric
+ ? getBreakPointColorScaler(
+ { ...fd, opacity: fd.opacity ?? 100 } as unknown as BucketsWithColorScale,
+ data,
+ accessor,
+ )
+ : () => fixedColor;
+ break;
+ }
+ case COLOR_SCHEME_TYPES.color_breakpoints: {
+ const colorBreakpoints = fd.color_breakpoints || [];
+ getFillColor = (d: JsonObject) => {
+ const breakpointIndex = getColorForBreakpoints(
+ accessor,
+ d as unknown as number[],
+ colorBreakpoints,
+ );
+ const breakpointColor =
+ breakpointIndex !== undefined &&
+ colorBreakpoints[breakpointIndex - 1]?.color;
+ if (breakpointColor) {
+ return [
+ breakpointColor.r,
+ breakpointColor.g,
+ breakpointColor.b,
+ 255,
+ ];
+ }
+ if (defaultBreakpointColor) {
+ return [
+ defaultBreakpointColor.r,
+ defaultBreakpointColor.g,
+ defaultBreakpointColor.b,
+ defaultBreakpointColor.a * 255,
+ ];
+ }
+ return [
+ DEFAULT_DECKGL_COLOR.r,
+ DEFAULT_DECKGL_COLOR.g,
+ DEFAULT_DECKGL_COLOR.b,
+ DEFAULT_DECKGL_COLOR.a * 255,
+ ];
+ };
+ break;
+ }
+ default: {
+ getFillColor = () => fixedColor;
+ }
+ }
+
+ const safeGetFillColor = (d: JsonObject): Color =>
+ getFillColor(d) || TRANSPARENT_COLOR_ARRAY;
+
+ const tooltipContentGenerator = createTooltipContent(
+ fd,
+ defaultTooltipContent(fd),
+ );
+
+ return new H3HexagonLayer({
+ id: `h3-hexagon-layer-${fd.slice_id}`,
+ data,
+ extruded: Boolean(extruded),
+ coverage,
+ elevationScale: elevationScale,
+
+ getHexagon: (d: JsonObject) => d.hexagon,
+ getFillColor: safeGetFillColor,
+ getElevation: (d: JsonObject) => d.elevation || 0,
+
+ ...commonLayerProps({
+ formData: fd,
+ setDataMask,
+ setTooltip,
+ setTooltipContent: tooltipContentGenerator,
+ filterState,
+ onContextMenu,
+ emitCrossFilters,
+ }),
Review Comment:
**🟠 Architect Review — HIGH**
When dashboard cross-filtering is enabled (emitCrossFilters=true), H3Hexagon passes formData to commonLayerProps, whose click handler calls getCrossFilterDataMask; that helper requires start_spatial/end_spatial, spatial, line_column, or geojson, none of which exist on the H3 form data, so clicking a hexagon throws "No valid spatial configuration found in form data" at runtime instead of emitting filters.
**Suggestion:** Either extend crossFiltersDataMask/commonLayerProps to support H3 by deriving filters from the h3_index value, or explicitly disable emitCrossFilters for this viz (and/or guard the onClick path) until H3-specific cross-filter handling is implemented.
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20an%20%2A%2AArchitect%20%2F%20Logical%20Review%2A%2A%20comment%20left%20during%20a%20code%20review.%20These%20reviews%20are%20first-class%2C%20important%20findings%20%E2%80%94%20not%20optional%20suggestions.%20Do%20NOT%20dismiss%20this%20as%20a%20%27big%20architectural%20change%27%20just%20because%20the%20title%20says%20architect%20review%3B%20most%20of%20these%20can%20be%20resolved%20with%20a%20small%2C%20localized%20fix%20once%20the%20intent%20is%20understood.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fplugins%2Flegacy-preset-chart-deckgl%2Fsrc%2Flayers%2FH3Hexagon%2FH3Hexagon.tsx%0A%2A%2ALine%3A%2A%2A%20196%3A204%0A%2A%2AComment%3A%2A%2A%0A%09%2AHIGH%3A%20When%20dashboard%20cross-filtering%20is%20enabled%20%28emitCrossFilters%3Dtrue%29%2C%20H3Hexagon%20passes%20formData%20to%20commonLayerProps%2C%20whose%20click%20handler%20calls%20getCrossFilterDataMask%3B%20that%20helper%20requires%20start_spa
tial%2Fend_spatial%2C%20spatial%2C%20line_column%2C%20or%20geojson%2C%20none%20of%20which%20exist%20on%20the%20H3%20form%20data%2C%20so%20clicking%20a%20hexagon%20throws%20%22No%20valid%20spatial%20configuration%20found%20in%20form%20data%22%20at%20runtime%20instead%20of%20emitting%20filters.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AIf%20a%20suggested%20approach%20is%20provided%20above%2C%20use%20it%20as%20the%20authoritative%20instruction.%20If%20no%20explicit%20code%20suggestion%20is%20given%2C%20you%20MUST%20still%20draft%20and%20apply%20your%20own%20minimal%2C%20localized%20fix%20%E2%80%94%20do%20not%20punt%20back%20with%20%27no%20suggestion%20provided%2C%20review%20manually%27.%20Keep%20the%20change%20as%20small%20as%20possible%3A%20add%20a%20guard%20clause%2C%20gate%20on%20a%20loading%20state%2C%20reorder%20an%2
0await%2C%20wrap%20in%20a%20conditional%2C%20etc.%20Do%20not%20refactor%20surrounding%20code%20or%20expand%20scope%20beyond%20the%20finding.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20an%20%2A%2AArchitect%20%2F%20Logical%20Review%2A%2A%20comment%20left%20during%20a%20code%20review.%20These%20reviews%20are%20first-class%2C%20important%20findings%20%E2%80%94%20not%20optional%20suggestions.%20Do%20NOT%20dismiss%20this%20as%20a%20%27big%20architectural%20change%27%20just%20because%20the%20title%20says%20architect%20review%3B%20most%20of%20these%20can%20be%20resolved%20with%20a%20small%2C%20localized%20fix%20o
nce%20the%20intent%20is%20understood.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fplugins%2Flegacy-preset-chart-deckgl%2Fsrc%2Flayers%2FH3Hexagon%2FH3Hexagon.tsx%0A%2A%2ALine%3A%2A%2A%20196%3A204%0A%2A%2AComment%3A%2A%2A%0A%09%2AHIGH%3A%20When%20dashboard%20cross-filtering%20is%20enabled%20%28emitCrossFilters%3Dtrue%29%2C%20H3Hexagon%20passes%20formData%20to%20commonLayerProps%2C%20whose%20click%20handler%20calls%20getCrossFilterDataMask%3B%20that%20helper%20requires%20start_spatial%2Fend_spatial%2C%20spatial%2C%20line_column%2C%20or%20geojson%2C%20none%20of%20which%20exist%20on%20the%20H3%20form%20data%2C%20so%20clicking%20a%20hexagon%20throws%20%22No%20valid%20spatial%20configuration%20found%20in%20form%20data%22%20at%20runtime%20instead%20of%20emitting%20filters.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AIf%20a%2
0suggested%20approach%20is%20provided%20above%2C%20use%20it%20as%20the%20authoritative%20instruction.%20If%20no%20explicit%20code%20suggestion%20is%20given%2C%20you%20MUST%20still%20draft%20and%20apply%20your%20own%20minimal%2C%20localized%20fix%20%E2%80%94%20do%20not%20punt%20back%20with%20%27no%20suggestion%20provided%2C%20review%20manually%27.%20Keep%20the%20change%20as%20small%20as%20possible%3A%20add%20a%20guard%20clause%2C%20gate%20on%20a%20loading%20state%2C%20reorder%20an%20await%2C%20wrap%20in%20a%20conditional%2C%20etc.%20Do%20not%20refactor%20surrounding%20code%20or%20expand%20scope%20beyond%20the%20finding.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
**Path:** superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/H3Hexagon/H3Hexagon.tsx
**Line:** 196:204
**Comment:**
*HIGH: When dashboard cross-filtering is enabled (emitCrossFilters=true), H3Hexagon passes formData to commonLayerProps, whose click handler calls getCrossFilterDataMask; that helper requires start_spatial/end_spatial, spatial, line_column, or geojson, none of which exist on the H3 form data, so clicking a hexagon throws "No valid spatial configuration found in form data" at runtime instead of emitting filters.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Feat/h3 hexagon layer [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38028:
URL: https://github.com/apache/superset/pull/38028#issuecomment-3913786060
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(chart): h3 hexagon layer [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38028:
URL: https://github.com/apache/superset/pull/38028#issuecomment-4327263366
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(chart): h3 hexagon layer [superset]
Posted by "DamianPendrak (via GitHub)" <gi...@apache.org>.
DamianPendrak commented on PR #38028:
URL: https://github.com/apache/superset/pull/38028#issuecomment-4918710010
Hi @rusackas, @Boreto1213 - following up here. If it's helpful, I'm happy to pick up the rebase so this doesn't stay stuck
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(chart): h3 hexagon layer [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #38028:
URL: https://github.com/apache/superset/pull/38028#issuecomment-4988259051
@DamianPendrak thanks in advance on that! As I'm sure you noticed, there are some unresolved bot review comments that might warrant an audit as well before we merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(chart): h3 hexagon layer [superset]
Posted by "DamianPendrak (via GitHub)" <gi...@apache.org>.
DamianPendrak commented on PR #38028:
URL: https://github.com/apache/superset/pull/38028#issuecomment-4994990428
Hi, I rebased this onto current master and addressed the open review comments. Since I can't push to this fork's branch directly, I've pushed my work to my fork and would appreciate you pulling it onto the PR branch @Boreto1213.
Branch: feat/h3-hexagon-layer on https://github.com/DamianPendrak/superset
What changed:
- Rebased onto master - your 9 original commits are untouched. The plugin directory was renamed on master (legacy-preset-chart-deckgl → preset-chart-deckgl), so there's one completion commit that moves the H3 layer into the renamed plugin and adapts a couple of imports that moved upstream (t → @apache-superset/core/translation).
- Then 3 review-fix commits @rusackas:
- read h3 config from rawFormData in H3 transformProps - fixes the failing transformProps tests.
- emit cross-filter on H3 hexagon click - with emitCrossFilters on, clicking a hexagon threw No valid spatial configuration found in form data because getCrossFilterDataMask had no branch for h3_index. Added one that emits an equality filter on the H3 column.
- fit H3 viewport to full hexagon boundaries - auto-zoom used only the first boundary vertex of each hexagon, clipping edge cells; now uses all vertices like Polygon does.
@Boreto1213 you can use these commands in your fork to push the rebased version:
`git fetch https://github.com/DamianPendrak/superset.git feat/h3-hexagon-layer:rebased-h3`
`git push origin --force rebased-h3:feat/h3-hexagon-layer`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org