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 `fbabce0552`. Moved `getTypedFixedRadius` to
`layers/utils/metricUtils.ts` 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 `not region_format` with a conditional enum selection without
simplifying the operation. `fbabce0552` clarifies the flag as `exact_only`;
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 `fbabce0552`: renamed the keyword-only parameter to
`exact_only` in both geographic resolvers, updated the caller and docstrings,
and retained `matches` 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]
