refactor(colors): simplify theme exports, unify scheme naming, add getCategoricalSchemes

Addresses reviewer feedback on the @apache-superset/core/theme color API:
use `export *` instead of an explicit re-export list, rename
ColorSchemeConfig/SequentialSchemeConfig to CategoricalScheme/SequentialScheme
(dropping the now-redundant duplicate CategoricalScheme interface), and add
getCategoricalSchemes() so extensions can fetch full scheme metadata without
a second round-trip through getSchemeColors per name.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

superset-frontend/packages/superset-core/src/theme/colors.test.ts

@@ -16,16 +16,11 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import {
-  URL_PARAMS,
-  RESERVED_CHART_URL_PARAMS,
-  RESERVED_DASHBOARD_URL_PARAMS,
-} from 'src/constants';
 
-test('permalinkKey is reserved on both the chart and dashboard URL param lists', () => {
-  // Dashboard and explore permalinks resolve against different backend
-  // KV resources/salts, so a key from one must never leak into the other's
-  // URL via the reserved-params passthrough logic.
-  expect(RESERVED_DASHBOARD_URL_PARAMS).toContain(URL_PARAMS.permalinkKey.name);
-  expect(RESERVED_CHART_URL_PARAMS).toContain(URL_PARAMS.permalinkKey.name);
+import { ColorSchemeGroup } from '@apache-superset/core/theme';
+
+test('ColorSchemeGroup has the expected string values', () => {
+  expect(ColorSchemeGroup.Custom).toBe('custom');
+  expect(ColorSchemeGroup.Featured).toBe('featured');
+  expect(ColorSchemeGroup.Other).toBe('other');
 });

superset-frontend/packages/superset-core/src/theme/colors.ts

@@ -0,0 +1,81 @@
+/**
+ * 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.
+ */
+
+/**
+ * Grouping/tier for a color scheme — controls how it appears in the
+ * scheme picker UI (e.g. Featured palettes are shown first).
+ *
+ * Mirrors @superset-ui/core's ColorSchemeGroup; kept here so
+ * palette configs have no dependency on @superset-ui/core.
+ */
+export enum ColorSchemeGroup {
+  Custom = 'custom',
+  Featured = 'featured',
+  Other = 'other',
+}
+
+/** Plain configuration object for a categorical color scheme. */
+export interface CategoricalScheme {
+  id: string;
+  label?: string;
+  colors: string[];
+  description?: string;
+  isDefault?: boolean;
+  group?: ColorSchemeGroup;
+}
+
+/**
+ * A sequential / diverging color scheme. The only difference from a
+ * categorical scheme is the optional `isDiverging` flag.
+ */
+export interface SequentialScheme extends CategoricalScheme {
+  isDiverging?: boolean;
+}
+
+/**
+ * Minimal interface for the categorical color scheme registry.
+ * Mirrors the public surface of @superset-ui/core's ColorSchemeRegistry.
+ */
+export interface CategoricalSchemeRegistryLike {
+  keys(): string[];
+  get(name: string): CategoricalScheme | null | undefined;
+}
+
+/**
+ * Returns an alphabetically sorted list of all registered categorical color
+ * scheme names. The host app provides the implementation via
+ * window.superset.theme.
+ */
+export declare function getCategoricalSchemeNames(): string[];
+
+/**
+ * Returns the full list of registered categorical color schemes (id, colors,
+ * and any other available metadata), sorted alphabetically by id. Prefer
+ * this over `getCategoricalSchemeNames` when scheme metadata (label,
+ * description, etc.) is needed, since extracting just the names would
+ * require a second round-trip through `getSchemeColors` per scheme.
+ * The host app provides the implementation via window.superset.theme.
+ */
+export declare function getCategoricalSchemes(): CategoricalScheme[];
+
+/**
+ * Returns the color array for a named scheme, or null if not found.
+ * The host app provides the implementation via window.superset.theme.
+ */
+export declare function getSchemeColors(schemeName: string): string[] | null;

superset-frontend/packages/superset-core/src/theme/index.tsx

@@ -88,3 +88,7 @@ export type {
 // Export theme utility functions
 export * from './utils/themeUtils';
 export * from './utils';
+
+// Color scheme API — types, enum, and declare-function bridge for extensions.
+// The host app provides the runtime implementations on window.superset.theme.
+export * from './colors';

superset-frontend/packages/superset-ui-core/test/color/colorSchemes.test.ts

@@ -24,6 +24,12 @@ import {
   CategoricalD3,
   CategoricalGoogle,
   CategoricalLyft,
+  CategoricalModernSunset,
+  CategoricalColorsOfRainbow,
+  CategoricalBlueToGreen,
+  CategoricalRedToYellow,
+  CategoricalWavesOfBlue,
+  CategoricalPresetSuperset,
   SequentialCommon,
   SequentialD3,
   CategoricalScheme,
@@ -41,22 +47,59 @@ describe('Color Schemes', () => {
         CategoricalLyft,
         CategoricalSuperset,
         CategoricalPreset,
+        CategoricalModernSunset,
+        CategoricalColorsOfRainbow,
+        CategoricalBlueToGreen,
+        CategoricalRedToYellow,
+        CategoricalWavesOfBlue,
+        CategoricalPresetSuperset,
       ].forEach(group => {
         expect(group).toBeInstanceOf(Array);
+        expect(group.length).toBeGreaterThan(0);
         group.forEach(scheme =>
           expect(scheme).toBeInstanceOf(CategoricalScheme),
         );
       });
     });
+
+    test('each scheme has a non-empty id and at least one color', () => {
+      [
+        ...CategoricalAirbnb,
+        ...CategoricalD3,
+        ...CategoricalEcharts,
+        ...CategoricalGoogle,
+        ...CategoricalLyft,
+        ...CategoricalPreset,
+        ...CategoricalSuperset,
+        ...CategoricalPresetSuperset,
+        ...CategoricalModernSunset,
+        ...CategoricalColorsOfRainbow,
+        ...CategoricalBlueToGreen,
+        ...CategoricalRedToYellow,
+        ...CategoricalWavesOfBlue,
+      ].forEach(scheme => {
+        expect(scheme.id).toBeTruthy();
+        expect(scheme.colors.length).toBeGreaterThan(0);
+      });
+    });
   });
+
   describe('sequential', () => {
     test('returns an array of SequentialScheme', () => {
       [SequentialCommon, SequentialD3].forEach(group => {
         expect(group).toBeInstanceOf(Array);
+        expect(group.length).toBeGreaterThan(0);
         group.forEach(scheme =>
           expect(scheme).toBeInstanceOf(SequentialScheme),
         );
       });
     });
+
+    test('each scheme has a non-empty id and at least two colors', () => {
+      [...SequentialCommon, ...SequentialD3].forEach(scheme => {
+        expect(scheme.id).toBeTruthy();
+        expect(scheme.colors.length).toBeGreaterThanOrEqual(2);
+      });
+    });
   });
 });

superset-frontend/src/constants.ts

@@ -123,7 +123,6 @@ export const RESERVED_CHART_URL_PARAMS: string[] = [
   URL_PARAMS.datasourceId.name,
   URL_PARAMS.datasourceType.name,
   URL_PARAMS.datasetId.name,
-  URL_PARAMS.permalinkKey.name,
   URL_PARAMS.versionHistory.name,
 ];
 export const RESERVED_DASHBOARD_URL_PARAMS: string[] = [

superset-frontend/src/core/index.ts

@@ -36,5 +36,6 @@ export * from './models';
 export * from './navigation';
 export * from './sqlLab';
 export * from './storage';
+export * from './theme';
 export * from './utils';
 export * from './views';

superset-frontend/src/core/theme/colors.ts

@@ -0,0 +1,52 @@
+/**
+ * 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 { theme as themeApi } from '@apache-superset/core';
+import { getCategoricalSchemeRegistry } from '@superset-ui/core';
+import type {
+  CategoricalScheme,
+  CategoricalSchemeRegistryLike,
+} from '@apache-superset/core/theme';
+
+const getRegistry = () =>
+  getCategoricalSchemeRegistry() as CategoricalSchemeRegistryLike | null;
+
+const getCategoricalSchemeNames: typeof themeApi.getCategoricalSchemeNames =
+  () => (getRegistry()?.keys() ?? []).sort();
+
+const getCategoricalSchemes: typeof themeApi.getCategoricalSchemes = () => {
+  const registry = getRegistry();
+  return getCategoricalSchemeNames()
+    .map(name => registry?.get(name))
+    .filter((scheme): scheme is CategoricalScheme => scheme != null);
+};
+
+const getSchemeColors: typeof themeApi.getSchemeColors = schemeName =>
+  getRegistry()?.get(schemeName)?.colors ?? null;
+
+/**
+ * Host implementation of the @apache-superset/core/theme color API.
+ * Spreads the contract namespace (types, enum, styling helpers) and supplies
+ * the runtime implementations for the declare-only registry bridge functions.
+ */
+export const theme: typeof themeApi = {
+  ...themeApi,
+  getCategoricalSchemeNames,
+  getCategoricalSchemes,
+  getSchemeColors,
+};

superset-frontend/src/core/theme/index.ts

@@ -0,0 +1,19 @@
+/**
+ * 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.
+ */
+export * from './colors';

superset-frontend/src/extensions/ExtensionsStartup.tsx

@@ -31,6 +31,7 @@ import {
   navigation,
   useNavigationTracker,
   sqlLab,
+  theme,
   views,
 } from 'src/core';
 import { useSelector } from 'react-redux';
@@ -64,6 +65,7 @@ const ExtensionsStartup: React.FC<{ children?: React.ReactNode }> = ({
       menus,
       navigation,
       sqlLab,
+      theme,
       views,
     };
 

superset-frontend/src/extensions/Namespaces.ts

@@ -36,6 +36,7 @@ import type {
   menus,
   navigation,
   sqlLab,
+  theme,
   views,
 } from 'src/core';
 
@@ -50,6 +51,7 @@ export interface Namespaces {
   menus: typeof menus;
   navigation: typeof navigation;
   sqlLab: typeof sqlLab;
+  theme: typeof theme;
   views: typeof views;
 }
 

superset/common/query_object.py

@@ -17,6 +17,7 @@
 # pylint: disable=invalid-name
 from __future__ import annotations
 
+import inspect
 import logging
 from datetime import datetime
 from pprint import pformat
@@ -205,8 +206,86 @@ class QueryObject:  # pylint: disable=too-many-instance-attributes
     def _set_post_processing(
         self, post_processing: list[dict[str, Any] | None] | None
     ) -> None:
-        post_processing = post_processing or []
-        self.post_processing = [post_proc for post_proc in post_processing if post_proc]
+        self.post_processing = [
+            self._drop_unsupported_options(post_proc)
+            for post_proc in post_processing or []
+            if post_proc
+        ]
+
+    @staticmethod
+    def _drop_unsupported_options(post_proc: dict[str, Any]) -> dict[str, Any]:
+        """
+        Drop options that the post-processing operation no longer accepts.
+
+        A chart's ``query_context`` is written when the chart is saved and is
+        never rewritten afterwards, while Explore rebuilds the query from
+        ``form_data`` at every render. A chart saved by an older version of
+        Superset can therefore reference an option that has since been removed
+        from the operation. ``exec_post_processing`` passes the stored options
+        as keyword arguments, so that option raises a bare ``TypeError`` on
+        every path that replays the stored ``query_context`` -- the chart data
+        endpoint, alerts and reports, thumbnails, CSV export -- while the same
+        chart still renders correctly in Explore.
+
+        Comparing against the signature avoids a hard-coded list of removed
+        option names, which would need extending at each release.
+        """
+        operation = post_proc.get("operation")
+        function = (
+            getattr(pandas_postprocessing, operation, None)
+            if isinstance(operation, str)
+            else None
+        )
+        if function is None:
+            # A missing or unknown operation is left untouched, so that
+            # exec_post_processing reports it as InvalidPostProcessingError.
+            return post_proc
+
+        parameters = inspect.signature(function).parameters
+        if any(
+            parameter.kind is inspect.Parameter.VAR_KEYWORD
+            for parameter in parameters.values()
+        ):
+            return post_proc
+
+        # `exec_post_processing` calls the operation as `operation(df, **options)`,
+        # so an option can only reach a parameter that a caller may fill by
+        # keyword. That excludes the first parameter, which receives the
+        # DataFrame positionally, and any positional-only or `*args` parameter.
+        keyword_parameters = {
+            name
+            for position, (name, parameter) in enumerate(parameters.items())
+            if position > 0
+            and parameter.kind
+            in (
+                inspect.Parameter.POSITIONAL_OR_KEYWORD,
+                inspect.Parameter.KEYWORD_ONLY,
+            )
+        }
+
+        options = post_proc.get("options") or {}
+        unsupported = {key for key in options if key not in keyword_parameters}
+        if not unsupported:
+            return post_proc
+
+        # Logged at info: a chart saved before the option was removed hits this
+        # on every render, so a warning would repeat for as long as the chart
+        # is not resaved, without anything new to report.
+        logger.info(
+            "Dropping unsupported option(s) %s of post-processing operation "
+            "`%s`. The chart's stored query_context predates the current "
+            "signature of that operation.",
+            sorted(unsupported),
+            operation,
+        )
+        return {
+            **post_proc,
+            "options": {
+                key: value
+                for key, value in options.items()
+                if key in keyword_parameters
+            },
+        }
 
     def _init_series_columns(
         self,

superset/utils/pandas_postprocessing/utils.py

@@ -15,7 +15,7 @@
 # specific language governing permissions and limitations
 # under the License.
 from collections.abc import Sequence
-from functools import partial
+from functools import partial, wraps
 from typing import Any, Callable
 
 import numpy as np
@@ -122,6 +122,10 @@ def scalar_to_sequence(val: Any) -> Sequence[str]:
 
 def validate_column_args(*argnames: str) -> Callable[..., Any]:
     def wrapper(func: Callable[..., Any]) -> Callable[..., Any]:
+        # `wraps` keeps `func` reachable through `__wrapped__`, so that
+        # `inspect.signature` reports the parameters of the decorated operation
+        # rather than the `(df, **options)` of this wrapper.
+        @wraps(func)
         def wrapped(df: DataFrame, **options: Any) -> Any:
             if _is_multi_index_on_columns(df):
                 # MultiIndex column validate first level

tests/unit_tests/pandas_postprocessing/test_utils.py

@@ -14,7 +14,13 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-from superset.utils.pandas_postprocessing import escape_separator, unescape_separator
+import inspect
+
+from superset.utils.pandas_postprocessing import (
+    escape_separator,
+    pivot,
+    unescape_separator,
+)
 
 
 def test_escape_separator():
@@ -28,3 +34,19 @@ def test_escape_separator():
     escape_string = escape_separator("hello,world")
     assert escape_string == r"hello\,world"
     assert unescape_separator(escape_string) == "hello,world"
+
+
+def test_validate_column_args_preserves_signature():
+    """
+    The decorator must not hide the signature of the operation it wraps.
+
+    `inspect.signature` follows `__wrapped__`, which `functools.wraps` sets.
+    Without it every decorated operation reports `(df, **options)`, and code
+    that inspects the signature -- see `QueryObject._drop_unsupported_options`
+    -- cannot tell a supported option from an unsupported one.
+    """
+    parameters = inspect.signature(pivot).parameters
+
+    assert pivot.__name__ == "pivot"
+    assert "options" not in parameters
+    assert {"index", "aggregates", "columns"} <= set(parameters)

tests/unit_tests/queries/query_object_test.py

@@ -22,6 +22,7 @@ from superset.common.query_object import QueryObject
 from superset.connectors.sqla.models import SqlaTable
 from superset.models.core import Database
 from superset.superset_typing import Metric
+from superset.utils import pandas_postprocessing
 from superset.utils.core import override_user
 
 
@@ -438,3 +439,143 @@ def test_cache_key_cache_impersonation_on_with_different_user_and_db_impersonati
         ],
         any_order=True,
     )
+
+
+def test_post_processing_drops_unsupported_options():
+    """
+    An option that the operation no longer accepts is dropped, not passed on.
+
+    A chart saved by an older version of Superset stores `flatten_columns` in
+    the options of its `pivot` operation. `pivot` lost that parameter when
+    flattening became its own operation, so replaying the stored query_context
+    raised `TypeError: pivot() got an unexpected keyword argument
+    'flatten_columns'`.
+    """
+    query_object = QueryObject(
+        row_limit=1,
+        post_processing=[
+            {
+                "operation": "pivot",
+                "options": {
+                    "index": ["__timestamp"],
+                    "columns": ["genre"],
+                    "aggregates": {"count": {"operator": "mean"}},
+                    "drop_missing_columns": False,
+                    "flatten_columns": True,
+                    "reset_index": True,
+                },
+            }
+        ],
+    )
+
+    options = query_object.post_processing[0]["options"]
+    assert "flatten_columns" not in options
+    assert "reset_index" not in options
+    assert options["drop_missing_columns"] is False
+    assert options["index"] == ["__timestamp"]
+
+
+def test_post_processing_keeps_supported_options():
+    """Options the operation accepts are left alone."""
+    post_processing = [
+        {
+            "operation": "pivot",
+            "options": {"index": ["__timestamp"], "aggregates": {}},
+        }
+    ]
+    query_object = QueryObject(row_limit=1, post_processing=post_processing)
+
+    assert query_object.post_processing == post_processing
+
+
+def test_post_processing_keeps_unknown_operation():
+    """
+    An unknown operation is kept, so that `exec_post_processing` can report it
+    as an `InvalidPostProcessingError` rather than being silently dropped here.
+    """
+    query_object = QueryObject(
+        row_limit=1,
+        post_processing=[{"operation": "does_not_exist", "options": {"a": 1}}, None],
+    )
+
+    assert query_object.post_processing == [
+        {"operation": "does_not_exist", "options": {"a": 1}}
+    ]
+
+
+def test_post_processing_drops_the_dataframe_parameter():
+    """
+    The DataFrame parameter is not an option.
+
+    `exec_post_processing` calls `operation(df, **options)`, so an option named
+    after the first parameter would raise `TypeError: pivot() got multiple
+    values for argument 'df'`.
+    """
+    query_object = QueryObject(
+        row_limit=1,
+        post_processing=[
+            {
+                "operation": "pivot",
+                "options": {"df": "malformed", "index": ["a"], "aggregates": {}},
+            }
+        ],
+    )
+
+    options = query_object.post_processing[0]["options"]
+    assert "df" not in options
+    assert options["index"] == ["a"]
+
+
+def test_post_processing_keeps_options_of_a_variadic_operation():
+    """An operation that accepts `**kwargs` accepts every option."""
+
+    def variadic(df, **kwargs):
+        return df
+
+    post_processing = [{"operation": "variadic", "options": {"anything": 1}}]
+    with patch.object(pandas_postprocessing, "variadic", variadic, create=True):
+        query_object = QueryObject(row_limit=1, post_processing=post_processing)
+
+    assert query_object.post_processing == post_processing
+
+
+def test_post_processing_drops_a_variadic_positional_option():
+    """
+    A `*args` parameter cannot be filled by a keyword argument.
+
+    `exec_post_processing` calls the operation as `operation(df, **options)`,
+    so an option named after a `*args` parameter would raise `TypeError:
+    variadic_positional() got an unexpected keyword argument 'args'` even
+    though the name appears in the signature.
+    """
+
+    def variadic_positional(df, *args, index=None):  # pylint: disable=unused-argument
+        return df
+
+    with patch.object(
+        pandas_postprocessing, "variadic_positional", variadic_positional, create=True
+    ):
+        query_object = QueryObject(
+            row_limit=1,
+            post_processing=[
+                {
+                    "operation": "variadic_positional",
+                    "options": {"args": [1], "index": ["a"]},
+                }
+            ],
+        )
+
+    options = query_object.post_processing[0]["options"]
+    assert "args" not in options
+    assert options["index"] == ["a"]
+
+
+def test_post_processing_keeps_an_entry_without_an_operation():
+    """
+    An entry that names no operation is kept, so that `exec_post_processing`
+    reports it as an `InvalidPostProcessingError`.
+    """
+    post_processing = [{"options": {"a": 1}}]
+    query_object = QueryObject(row_limit=1, post_processing=post_processing)
+
+    assert query_object.post_processing == post_processing