aminghadersohi commented on code in PR #44739:
URL: https://github.com/apache/superset/pull/44739#discussion_r4129383928
##########
superset/dataframe.py:
##########
@@ -72,7 +86,9 @@ def df_to_records(dframe: pd.DataFrame) -> list[dict[str,
Any]]:
for record in records:
for key in record:
record[key] = (
- None if _is_na(record[key]) else
_convert_big_integers(record[key])
+ None
+ if _is_na(record[key])
+ else _convert_decimals(_convert_big_integers(record[key]))
)
Review Comment:
That cache is not JSON. cache_manager.data_cache is SupersetCache(Cache)
from Flask-Caching, which only overrides the key hash and pickles values, so a
Decimal in the list-of-dicts round-trips through
_store_in_cache/_get_from_cache unchanged. That executor is also the
Database.execute() API, a separate path from the SQL Lab worker that
df_to_records serves.
##########
tests/unit_tests/dataframe_test.py:
##########
@@ -363,3 +365,44 @@ def
test_df_to_records_with_json_serialization_like_sql_lab() -> None:
)
parsed_no_flag = superset_json.loads(json_str_no_flag)
assert parsed_no_flag == parsed # Same result
+
+
[email protected](
+ "value",
+ [
+ "12345678901234567890.123456789012345678",
+ "-12345678901234567890.123456789012345678",
+ "0.000000000000000001",
+ "10.50",
+ "0.00",
+ "1E+30",
+ ],
+)
+def test_decimal_records_keep_all_digits(value: str) -> None:
Review Comment:
There is no BITO.md in this repo, and the surrounding tests in
dataframe_test.py (test_df_to_records, test_js_max_int and the rest) carry no
docstrings either, so this would be the odd file out. Leaving it matching the
file.
##########
tests/unit_tests/dataframe_test.py:
##########
@@ -363,3 +365,44 @@ def
test_df_to_records_with_json_serialization_like_sql_lab() -> None:
)
parsed_no_flag = superset_json.loads(json_str_no_flag)
assert parsed_no_flag == parsed # Same result
+
+
[email protected](
+ "value",
+ [
+ "12345678901234567890.123456789012345678",
+ "-12345678901234567890.123456789012345678",
+ "0.000000000000000001",
+ "10.50",
+ "0.00",
+ "1E+30",
+ ],
+)
+def test_decimal_records_keep_all_digits(value: str) -> None:
+ decimal = Decimal(value)
Review Comment:
The file imports 'from decimal import Decimal' and never 'import decimal',
so nothing is shadowed at runtime. The local is also self-evidently a Decimal
from the constructor on the same line.
--
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]
