aminghadersohi commented on code in PR #44148:
URL: https://github.com/apache/superset/pull/44148#discussion_r4192117076
##########
superset-frontend/plugins/preset-chart-deckgl/src/layers/Scatter/buildQuery.ts:
##########
@@ -48,6 +48,23 @@ export interface DeckScatterFormData
max_radius?: number;
color_picker?: { r: number; g: number; b: number; a: number };
dimension?: string;
+ mcp_geographic?: boolean;
+}
+
+/**
+ * Typed MCP compatibility reads a bare numeric-string radius as a fixed size.
+ * Native charts interpret every bare string as a saved metric name.
+ */
+export function getTypedFixedRadius(
+ formData: Pick<DeckScatterFormData, 'mcp_geographic' | 'point_radius_fixed'>,
+): number | null {
+ const { mcp_geographic, point_radius_fixed } = formData;
+ return mcp_geographic &&
+ typeof point_radius_fixed === 'string' &&
+ point_radius_fixed.trim() !== '' &&
+ Number.isFinite(Number(point_radius_fixed))
+ ? Number(point_radius_fixed)
+ : null;
}
Review Comment:
Fixed in . Moved to and updated both Scatter consumers. Its input is a
structural object, so the shared utility does not depend on the query-builder
module. Runtime behavior is unchanged. Validation: Scatter and metric-utils
Jest tests: 97 passed across 4 suites; deck.gl TypeScript build and all
applicable changed-file pre-commit hooks passed.
##########
superset/utils/geographic.py:
##########
@@ -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.
+
+"""Exact-first, ambiguity-safe resolution against rendered geographic
identifiers."""
+
+import unicodedata
+from collections.abc import Iterable
+
+from superset.utils.geographic_regions import REGIONS
+
+
+def geographic_key(value: str) -> str:
+ """Fold case and diacritics, but never guess or perform fuzzy matching."""
+ return "".join(
+ c
+ for c in unicodedata.normalize("NFD", value.lower())
+ if not unicodedata.combining(c)
+ )
+
+
+def resolve_geographic_value(
+ value: object,
+ entries: Iterable[tuple[str, str]],
+ *,
+ fold_diacritics: bool = True,
+ exact: bool = False,
Review Comment:
Declined the enum change. This keyword-only flag controls one operation:
whether to disable folded fallback after an exact lookup. Both paths use the
same nonempty-string validation, ambiguity check, and error handling; an enum
would not relocate or improve that handling. The renderer must still choose its
behavior based on whether an explicit region format is present, so an enum
would replace with a conditional enum selection without simplifying the
operation. clarifies the flag as ; diacritic folding remains a separate
format-specific policy. Geographic/MCP pytest coverage: 401 passed, including
legacy exact-boundary regressions. Keeping the boolean avoids unnecessary
enum/caller churn.
##########
superset/utils/geographic.py:
##########
@@ -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.
+
+"""Exact-first, ambiguity-safe resolution against rendered geographic
identifiers."""
+
+import unicodedata
+from collections.abc import Iterable
+
+from superset.utils.geographic_regions import REGIONS
+
+
+def geographic_key(value: str) -> str:
+ """Fold case and diacritics, but never guess or perform fuzzy matching."""
+ return "".join(
+ c
+ for c in unicodedata.normalize("NFD", value.lower())
+ if not unicodedata.combining(c)
+ )
+
+
+def resolve_geographic_value(
+ value: object,
+ entries: Iterable[tuple[str, str]],
+ *,
+ fold_diacritics: bool = True,
+ exact: bool = False,
+) -> str:
+ """Resolve exactly first, then accept only a unique folded identifier.
+
+ With ``exact``, only identical identifiers resolve.
+ """
+ if not isinstance(value, str) or not value or len(value) > 500:
+ raise ValueError(
+ "Geographic values must be nonempty strings of at most 500
characters"
+ )
+ key = geographic_key if fold_diacritics else str.lower
+ pairs = list(entries)
+ matches = {code for alias, code in pairs if alias == value}
+ if not matches and not exact:
Review Comment:
Addressed in : renamed the keyword-only parameter to in both geographic
resolvers, updated the caller and docstrings, and retained for the candidate
set. The deleted set name did not create runtime shadowing, but the rename
makes the boolean intent clearer and avoids confusion with exact-match
candidates. Resolution behavior is unchanged. Geographic/MCP pytest tests: 401
passed; all applicable changed-file pre-commit hooks passed.
--
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]
