aminghadersohi commented on PR #40339:
URL: https://github.com/apache/superset/pull/40339#issuecomment-4559406342
Correction to my earlier reply on `362b9faaab` — that commit introduced a
latent bug in the `point_radius_fixed` bare-string path that is now fixed in
`06cea32872`.
---
**Full status of your two items from review `4375664950` (20:03 UTC):**
**1. String-backed metrics (`size: "count"`, `metric: "sum__revenue"`)** —
fixed in `362b9faaab` via `_is_metric_ref()`, which accepts non-numeric strings
and rejects numeric scalars (`"100"`). The `size` and `metric` fields now pass
through string metric keys correctly.
**2. `deck_geojson IS NOT NULL` filter** — fixed in `362b9faaab`.
`_deck_gl_null_filters()` iterates over `("line_column", "geojson")` and adds
an `IS NOT NULL` filter for the `geojson` column when it is a non-empty string,
mirroring `BaseDeckGLViz.add_null_filters()` behavior.
---
**For review `4375780664` (20:20 UTC) — `point_radius_fixed` bare string:**
Fixed in `e32a683f6f`, but that commit had a bug: `elif _is_metric_ref(prf)`
fired for any dict (including `{"type": "fix", "value": 100}`) because
`_is_metric_ref` returns `True` for all dicts. This would have incorrectly
included a fixed pixel radius dict as a SQL metric.
Corrected in `06cea32872` — the branch is now `elif isinstance(prf, str) and
_is_metric_ref(prf)`, so only bare non-numeric strings (legacy `"count"`) are
appended; fixed-pixel dicts and numeric strings are still excluded.
--
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]
