aminghadersohi commented on PR #43569: URL: https://github.com/apache/superset/pull/43569#issuecomment-5612400411
Re-reviewed `458631c26eb4515be0f6582f353523a409c16a90` against `8d0895e9`. Thanks for the refresh: **update-preview casing is fixed**, and both update paths preserve omitted `show_labels`/`number_format`. The remaining gaps from the previous review still reproduce: - **Ordering:** with `row_limit=1`, the shared query builder emits only metric-descending order when enabled, and no `orderby` when disabled. Both cases still omit the frontend Treemap hierarchy ordering (`chart_helpers.py:578–585`). - **Previews:** `region/product/SUM(revenue)` still renders through the saved/form-data-key scatter fallback (`get_chart_preview.py:575`) or an unsaved bar spec with only region color encoding, without a value axis or hierarchy (`preview_utils.py:953`). - **Round-trip and update defaults:** native string hierarchy columns and saved/SIMPLE/SQL metric shapes still fail request validation. Both update paths reset omitted `color_scheme='lyftColors'`, `row_limit=7`, and `sort_by_metric=false` to `supersetColors`, `100`, and `true`. The Treemap schema/mapper still don't expose the frontend presentation controls; the shared merge's omitted-field handling is specialized for Gauge (`chart_utils.py:675–726`). Validation: **1,578 chart tests passed, 1 skipped**, plus eight temporary diagnostic cases confirming the remaining gaps and the fixes above; Ruff and whitespace checks passed. The Treemap test file is unchanged and still lacks the requested product-path regression coverage. CI has no failing/pending checks and mergeability is clean, but those green checks don't cover these behaviors. Please address the remaining Treemap paths before considering 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]
