[GitHub] [superset] ktmud commented on a change in pull request #13434: fix(query): order by adhoc metrics should trigger group by

[GitBox <gi...@apache.org>]

ktmud commented on a change in pull request #13434:
URL: https://github.com/apache/superset/pull/13434#discussion_r589231487



##########
File path: superset/utils/core.py
##########
@@ -1599,6 +1599,22 @@ def find_duplicates(items: Iterable[InputType]) -> List[InputType]:
     return [item for item, count in collections.Counter(items).items() if count > 1]
 
 
+def remove_duplicates(
+    items: Iterable[InputType], key: Optional[Callable[[InputType], Any]] = None
+) -> List[InputType]:
+    """Remove duplicate items in an iterable."""
+    if not key:
+        return list(dict.fromkeys(items).keys())
+    seen = set()
+    result = []
+    for item in items:
+        item_key = key(item)
+        if item_key not in seen:
+            seen.add(item_key)
+            result.append(item)
+    return result

Review comment:
       This function is not used by this PR anymore, but I'm keeping it here for convenience.

##########
File path: superset/common/query_object.py
##########
@@ -166,11 +166,11 @@ def __init__(
         #   1. 'metric_name'   - name of predefined metric
         #   2. { label: 'label_name' }  - legacy format for a predefined metric
         #   3. { expressionType: 'SIMPLE' | 'SQL', ... } - adhoc metric
-        self.metrics = [
-            metric
-            if isinstance(metric, str) or "expressionType" in metric
-            else metric["label"]  # type: ignore
-            for metric in metrics
+        self.metrics = metrics and [
+            x

Review comment:
       Use `x` in case some may wonder whether `metric` is also from the parent context (e.g. an `__init__` argument).




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org