bito-code-review[bot] commented on code in PR #44739:
URL: https://github.com/apache/superset/pull/44739#discussion_r4129385295
##########
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:
<!-- Bito Reply -->
The reviewer's suggestion to add docstrings to the new test functions is a
standard practice for improving code maintainability and clarity. While you
noted that other tests in the file lack docstrings, adding them to your new
tests is a positive step that aligns with best practices for new code
contributions. Applying this suggestion will make the intent of your new tests
immediately clear to other developers.
**tests/unit_tests/dataframe_test.py**
```
@pytest.mark.parametrize(
"value",
[
"12345678901234567890.123456789012345678",
"-12345678901234567890.123456789012345678",
"0.000000000000000001",
"10.50",
"0.00",
"1E+30",
],
)
def test_decimal_records_keep_all_digits(value: str) -> None:
"""Test that decimal values are preserved as strings without precision
loss."""
```
##########
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:
<!-- Bito Reply -->
The suggestion to rename the variable is not strictly necessary because the
local variable does not shadow the module name in this context. You may choose
to ignore this suggestion, as the code is clear and correct as written.
**tests/unit_tests/dataframe_test.py**
```
def test_decimal_records_keep_all_digits(value: str) -> None:
decimal = Decimal(value)
```
--
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]
