aminghadersohi commented on PR #43569: URL: https://github.com/apache/superset/pull/43569#issuecomment-5574547463
Re-reviewed `8d0895e986ef786c7b901385e39928e5424b232b` against the previous head and refreshed base, also checking integration with master `53070fa4`. The range-diff is effectively a rebase; the previously requested Treemap gaps remain: - **Ordering is partially fixed by the refreshed base:** `chart_helpers.py:508–517` emits metric-descending order, but omits the hierarchy tie-breakers. With `row_limit=1`, `sort_by_metric=false` still emits no `orderby`; the frontend Treemap `buildQuery` appends the ordered hierarchy in both cases. - **Previews still aren't Treemaps:** for `region/product/SUM(revenue)` data, the saved/form-data-key strategy falls through to scatter (`get_chart_preview.py:488–531`); the unsaved helper emits a bar with only a region color encoding, no value axis or hierarchy (`preview_utils.py:470–566`). Please share a real Treemap spec across both paths. - **Native round-trip/preservation remains missing:** request validation rejects native string `groupby` and saved/SIMPLE/SQL singular metric shapes. A replacement saved update drops omitted `show_labels`/`number_format` and resets `lyftColors` to `supersetColors` (`update_chart.py:233–252`); update-preview likewise resets omitted presentation values. The schema/mapper still lack the frontend presentation controls. - **Update-preview casing:** lowercase hierarchy/metric names reach `validate_and_compile` unchanged against uppercase dataset columns (`update_chart_preview.py:220–274`), unlike generate/update normalization. Validation: all **1,403 MCP chart tests pass**, as do Ruff on the six PR files and `git diff --check`; focused temporary reproductions expose the above behavior. The PR still adds only schema/mapper/registry Treemap tests, not the requested generate/update/update-preview/get-chart-data/preview workflow regressions. Please cover these behaviors before considering Treemap support complete. No contributor-branch changes made. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
