codeant-ai-for-open-source[bot] commented on code in PR #40698:
URL: https://github.com/apache/superset/pull/40698#discussion_r3345919117
##########
superset/utils/pandas_postprocessing/geography.py:
##########
@@ -16,7 +16,13 @@
# under the License.
from typing import Optional
-import pygeohash as geohash_lib
+try:
+ import pygeohash as geohash_lib
+except ImportError:
+ try:
+ import geohash as geohash_lib
+ except ImportError:
+ geohash_lib = None
Review Comment:
**Suggestion:** The fallback import chain does not include the `geohash2`
module, so environments that only have `geohash2` installed still end up with
`geohash_lib = None` and fail at runtime in geohash encode/decode
post-processing. Add `geohash2` as an additional fallback import before
assigning `None`. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Geohash decode post-processing fails when only geohash2 installed.
- ❌ Geohash encode post-processing unavailable in such environments.
- ⚠️ Charts using geohash post-processing error with missing library.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Create a Python environment where only the `geohash2` package is
installed and neither
`pygeohash` nor `geohash` are available (e.g. `pip install geohash2` only).
Import
`superset.utils.pandas_postprocessing.geography`, which executes the
fallback import block
at `geography.py:19–25`, trying `import pygeohash as geohash_lib` then
`import geohash as
geohash_lib` and finally assigning `geohash_lib = None` when both imports
fail.
2. Call `geohash_decode()` exactly as in
`tests/unit_tests/pandas_postprocessing/test_geography.py:26–33`, passing
`df=lonlat_df[["city", "geohash"]]`, `geohash="geohash"`,
`latitude="latitude"`, and
`longitude="longitude"`. This mirrors how the helper would be used in
post-processing
pipelines.
3. Inside `geohash_decode()` at `geography.py:34–52`, the guard `if not
geohash_lib:`
(line 46) evaluates to `True` because `geohash_lib` is `None`, and the
function raises
`InvalidPostProcessingError("The 'pygeohash' library is required to decode
geohashes.
Please install pygeohash.")` instead of using the available `geohash2`
implementation.
4. Similarly, calling `geohash_encode()` as in `test_geohash_encode` at
`test_geography.py:45–52` exercises `geohash_encode()` in
`geography.py:65–105`, where the
guard at lines 80–86 again sees `geohash_lib` as `None` and raises
`InvalidPostProcessingError`, breaking geohash-encoding post-processing in
any context
when only `geohash2` is installed.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2fec03c423cc4975ae5ff6846bb20f75&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=2fec03c423cc4975ae5ff6846bb20f75&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/pandas_postprocessing/geography.py
**Line:** 19:25
**Comment:**
*Incomplete Implementation: The fallback import chain does not include
the `geohash2` module, so environments that only have `geohash2` installed
still end up with `geohash_lib = None` and fail at runtime in geohash
encode/decode post-processing. Add `geohash2` as an additional fallback import
before assigning `None`.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40698&comment_hash=ff009775492b01bcbce03ed73cfa3ad44085d053f06d2a3b309de873f27c1f05&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40698&comment_hash=ff009775492b01bcbce03ed73cfa3ad44085d053f06d2a3b309de873f27c1f05&reaction=dislike'>👎</a>
##########
superset/viz.py:
##########
@@ -35,7 +35,14 @@
import numpy as np
import pandas as pd
import polyline
-import pygeohash
+
+try:
+ import pygeohash
+except ImportError:
+ try:
+ import geohash as pygeohash # type: ignore[no-redef]
+ except ImportError:
+ pygeohash = None # type: ignore[assignment]
Review Comment:
**Suggestion:** This import compatibility block also omits `geohash2`, so
deployments that only provide that package still set `pygeohash` to `None` and
geohash-based deck visualizations will raise `SpatialException` when decoding
paths. Include a `geohash2` fallback module import here as well. [incomplete
implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Deck.gl path charts using geohash lines fail rendering.
- ❌ Deck.gl spatial charts with geohash columns raise SpatialException.
- ⚠️ Geospatial features unreliable when environment ships geohash2 only.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Use a Python environment where only the `geohash2` package is installed
and neither
`pygeohash` nor `geohash` are importable. Import `superset.viz`, which runs
the
compatibility block at `viz.py:39–45`, first trying `import pygeohash` then
`import
geohash as pygeohash` and finally setting the module-level `pygeohash =
None` when both
imports fail.
2. Configure a deck.gl chart that uses geohash-encoded spatial data: for
BaseDeckGLViz-based charts, supply form data with a spatial control of type
`"geohash"`
(e.g. `"geohash_key": {"type": "geohash", "geohashCol": "geo"}` as in
`tests/integration_tests/viz_tests.py:9–21`) so that
`BaseDeckGLViz.process_spatial_data_obj()` at `viz.py:1904–1923` takes the
`'geohash'`
branch and maps the geohash column through `self.reverse_geohash_decode`.
3. When `process_spatial_data_obj()` executes for this geohash spatial
config, it calls
`reverse_geohash_decode()` at `viz.py:1887–1896`. Because the module-level
`pygeohash` is
`None` from the import block (lines 39–45), the guard `if not pygeohash:` at
line 1888
raises `SpatialException("The 'pygeohash' library is required to decode
geohashes. Please
install pygeohash.")`, causing deck.gl spatial charts with geohash columns
to fail instead
of decoding with the available `geohash2`.
4. For `DeckPathViz` (defined at `viz.py:2223–2299`) with `viz_type =
"deck_path"`,
selecting `"line_type": "geohash"` in the form data causes `deser_map` at
lines 2230–2234
to choose `geohash_to_json()` at `viz.py:2197–2219` for path
deserialization; that
function immediately checks `if not pygeohash:` and, with `pygeohash` set to
`None` by the
import block, raises the same `SpatialException`, so deck.gl path or polygon
visualizations using geohash-encoded paths cannot render when only
`geohash2` is
installed.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=916a9f91f4c24c618c62726071418643&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=916a9f91f4c24c618c62726071418643&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/viz.py
**Line:** 39:45
**Comment:**
*Incomplete Implementation: This import compatibility block also omits
`geohash2`, so deployments that only provide that package still set `pygeohash`
to `None` and geohash-based deck visualizations will raise `SpatialException`
when decoding paths. Include a `geohash2` fallback module import here as well.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40698&comment_hash=88d1c98a4c6ef5f8b3aeba13da3b32f4286087b561f792b43ae78c5792fc1979&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40698&comment_hash=88d1c98a4c6ef5f8b3aeba13da3b32f4286087b561f792b43ae78c5792fc1979&reaction=dislike'>👎</a>
--
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]
