SEPURI-SAI-KRISHNA opened a new pull request, #45086:
URL: https://github.com/apache/superset/pull/45086
### SUMMARY
`geohash_decode` and `geodetic_parse` build their parsed frame as a bare
`DataFrame()`, so it gets a fresh `RangeIndex`. `_append_columns` aligns on the
index, so when the caller's frame is indexed by anything other than 0..n-1
nothing matches: the real rows get no coordinates and the parsed values arrive
as extra rows.
Both now build the frame on the caller's index:
```python
lonlat_df = DataFrame(index=df.index)
```
That makes the alignment a no-op instead of a mismatch. It is correct for
either branch of `_append_columns`, the overwrite one and the append one, so it
does not depend on #45083.
`geohash_encode` is left alone deliberately: it slices `df[[latitude,
longitude]]`, so it already inherits the caller's index. A test pins that it
stays correct.
The index has never been carried here. Before #19116 `_append_columns` was a
plain `assign`, which also aligns on the index, so this surfaced as silently
null coordinates with the right row count; #19116's `pd.concat(axis="columns")`
unions the index instead and turned that into extra rows as well. So this fixes
a null-coordinate bug that is older than the row-count one, and both come from
the same bare `DataFrame()`.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A: backend only.
A `pivot` followed by a `geodetic_parse`, which is the shortest chain that
hands these operations a non-`RangeIndex` frame:
```python
pivoted = pivot(df=df, index=["region"], columns=[],
aggregates={"geodetic": {"operator": "max"}})
geodetic_parse(df=pivoted, geodetic="geodetic", latitude="lat",
longitude="lon")
```
before, 2 rows in and 4 out:
```
geodetic lat lon
east 41.1 -73.5 NaN NaN
west 40.7 -74.0 NaN NaN
0 NaN 41.1 -73.5
1 NaN 40.7 -74.0
```
after:
```
geodetic lat lon
east 41.1 -73.5 41.1 -73.5
west 40.7 -74.0 40.7 -74.0
```
| operation | builds its own frame | affected |
|---|---|---|
| `geohash_decode` | yes, `DataFrame()` | fixed |
| `geodetic_parse` | yes, `DataFrame()` | fixed |
| `geohash_encode` | no, slices `df` | not affected, pinned by a new test |
A frame that already has a 0-based index is unchanged, which is why the
existing tests pass untouched: they all build from `lonlat_df`, which has one.
### TESTING INSTRUCTIONS
```bash
pytest tests/unit_tests/pandas_postprocessing/test_geography.py
```
Five cases are added, four of which fail on master:
| test | covers | on master |
|---|---|---|
| `test_geography_keeps_the_index_it_was_given[geohash_decode]` | a
`city`-indexed frame keeps its rows and gets its coordinates | fails |
| `...[geodetic_parse]` | the same for the geodetic parser | fails |
| `...[geodetic_parse-with-altitude]` | the three-column parse, where the
altitude is also mapped | fails |
| `test_geodetic_parse_after_pivot_keeps_one_row_per_group` | the `pivot`
chain above, end to end through two real operations | fails |
| `test_geohash_encode_keeps_the_index_it_was_given` | that the one
operation needing no change stays correct | passes, guards it |
The file goes from 5 tests to 10, and the five existing ones are unchanged.
The post-processing suite is 191 passed, up from 186.
`tests/unit_tests/{pandas_postprocessing,charts,queries}` run clean at 584
passed, 2 xfailed (both pre-existing). `ruff check`, `ruff format --check` and
`pre-commit run mypy` are clean, and `pylint --rcfile=.pylintrc` rates the
changed file 10.00/10.
I also ran this against #45083's branch, since both touch this area: 194
passed with the two changes together, so they compose.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #45085
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
