korbit-ai[bot] commented on code in PR #32739:
URL: https://github.com/apache/superset/pull/32739#discussion_r2002134395
##########
superset/utils/excel.py:
##########
@@ -56,10 +56,24 @@ def df_to_excel(df: pd.DataFrame, **kwargs: Any) -> Any:
def apply_column_types(
df: pd.DataFrame, column_types: list[GenericDataType]
) -> pd.DataFrame:
+ """
+ Applies the column types to the dataframe to prepare for an excel export
+
+ :param df: The dataframe to apply the column types to
+ :param column_types: The types of the columns
+ :return: The dataframe with the column types applied
+ """
for column, column_type in zip(df.columns, column_types, strict=False):
if column_type == GenericDataType.NUMERIC:
try:
df[column] = pd.to_numeric(df[column])
+ # if the number is too large, convert it to a string
+ # Excel does not support numbers larger than 10^15
+ df[column] = df[column].apply(
+ lambda x: str(x)
+ if isinstance(x, (int, float)) and abs(x) > 10**15
Review Comment:
### Undefined magic number constant <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Magic number 10**15 is used without being defined as a named constant.
###### Why this matters
Future maintainers may not immediately understand the significance of this
specific number without context.
###### Suggested change ∙ *Feature Preview*
```python
# At module level
EXCEL_MAX_NUMBER = 10**15
# In the function
df[column] = df[column].apply(
lambda x: str(x)
if isinstance(x, (int, float)) and abs(x) > EXCEL_MAX_NUMBER
else x
)
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e21791af-cedc-4e24-b5e7-7c3b1f0b3dc7/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e21791af-cedc-4e24-b5e7-7c3b1f0b3dc7?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e21791af-cedc-4e24-b5e7-7c3b1f0b3dc7?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e21791af-cedc-4e24-b5e7-7c3b1f0b3dc7?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e21791af-cedc-4e24-b5e7-7c3b1f0b3dc7)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:766a466f-d3f2-4dfd-bf08-cd09971edb2e -->
[](766a466f-d3f2-4dfd-bf08-cd09971edb2e)
##########
superset/utils/excel.py:
##########
@@ -56,10 +56,24 @@ def df_to_excel(df: pd.DataFrame, **kwargs: Any) -> Any:
def apply_column_types(
df: pd.DataFrame, column_types: list[GenericDataType]
) -> pd.DataFrame:
+ """
+ Applies the column types to the dataframe to prepare for an excel export
+
+ :param df: The dataframe to apply the column types to
+ :param column_types: The types of the columns
+ :return: The dataframe with the column types applied
+ """
for column, column_type in zip(df.columns, column_types, strict=False):
if column_type == GenericDataType.NUMERIC:
try:
df[column] = pd.to_numeric(df[column])
+ # if the number is too large, convert it to a string
+ # Excel does not support numbers larger than 10^15
+ df[column] = df[column].apply(
+ lambda x: str(x)
+ if isinstance(x, (int, float)) and abs(x) > 10**15
+ else x
+ )
Review Comment:
### Extract complex lambda to named function <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The lambda function used for handling large numbers could be extracted into
a named helper function for better readability and reusability.
###### Why this matters
Inline lambda functions can make code harder to understand and maintain,
especially when they contain business logic. A named function would make the
intention clearer and could be reused if needed.
###### Suggested change ∙ *Feature Preview*
```python
def convert_large_number_to_string(x: Any) -> Any:
"""Convert numbers larger than 10^15 to strings for Excel
compatibility."""
if isinstance(x, (int, float)) and abs(x) > 10**15:
return str(x)
return x
# Then in the code:
df[column] = df[column].apply(convert_large_number_to_string)
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1d1d4a4e-e90a-41d5-8b8b-9b91b3500e04/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1d1d4a4e-e90a-41d5-8b8b-9b91b3500e04?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1d1d4a4e-e90a-41d5-8b8b-9b91b3500e04?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1d1d4a4e-e90a-41d5-8b8b-9b91b3500e04?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1d1d4a4e-e90a-41d5-8b8b-9b91b3500e04)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:6c71f766-1b6e-4a42-9d40-1b5f3d630060 -->
[](6c71f766-1b6e-4a42-9d40-1b5f3d630060)
--
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]
