aminghadersohi commented on code in PR #45126:
URL: https://github.com/apache/superset/pull/45126#discussion_r4234655957
##########
superset/semantic_layers/mapper.py:
##########
@@ -192,7 +194,9 @@ def get_results(query_object: QueryObject) -> QueryResult:
# Add this query's requests to the collection
all_requests.extend(result.requests)
- offset_df = stringify_extension_columns(result.results).to_pandas()
+ offset_df = stringify_extension_columns(result.results).to_pandas(
+ integer_object_nulls=True,
+ )
Review Comment:
With a time offset, a category that has no prior-period row gets `NaN` from
the left join below, which upcasts an `int64` offset metric (one with no nulls
of its own) to `float64`. So `9007199254740993` still comes back as
`9007199254740992.0` in `amount__1 week ago`, contrary to the docs paragraph.
Casting integer offset metrics to `object` before the merge keeps them exact:
```suggestion
offset_df = stringify_extension_columns(result.results).to_pandas(
integer_object_nulls=True,
)
# The left join below adds NaN for unmatched rows, which would turn
# an int64 metric into float64; keep integer metrics as exact ints.
offset_df = offset_df.astype(
{
metric: object
for metric in metric_names
if metric in offset_df and offset_df[metric].dtype.kind ==
"i"
}
)
```
I checked this through `POST /api/v1/chart/data` with your mocked-provider
setup: main period `A`, `B`; offset period `A` only, value `9007199254740993`.
Head returns `9007199254740992.0`, and the merge base returns the same, so this
isn't a regression. With the suggestion applied it returns
`"9007199254740993"`, all 14 tests in `integer_precision_test.py` pass, and
`semantic_layers` coverage stays at 100%. Your offset fixture always gives both
periods the same categories and puts a null inside the offset result, which
keeps the column `object`. That's why it doesn't catch this. A case where the
offset result has no row for `B` would pin it.
--
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]
