Skip to content

Add string sanitization for Excel export#408

Merged
ImMin5 merged 1 commit intocloudforet-io:masterfrom
ImMin5:master
Apr 29, 2025
Merged

Add string sanitization for Excel export#408
ImMin5 merged 1 commit intocloudforet-io:masterfrom
ImMin5:master

Conversation

@ImMin5
Copy link
Copy Markdown
Member

@ImMin5 ImMin5 commented Apr 29, 2025

Category

  • New feature
  • Bug fix
  • Improvement
  • Refactor
  • etc

Description

  • add string sanitization for Excel export

Known issue

Signed-off-by: ImMin5 <mino@megazone.com>
@ImMin5 ImMin5 added the enhancement New feature or request label Apr 29, 2025
@ImMin5 ImMin5 requested a review from Copilot April 29, 2025 04:55
@ImMin5 ImMin5 self-assigned this Apr 29, 2025
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds string sanitization for Excel exports by removing illegal characters from strings before writing the DataFrame to Excel. Key changes include:

  • Adding a helper static method (_sanitize_string) to clean string values.
  • Integrating the sanitization step into the Excel export process.
  • Adjusting import order to support the new functionality.

Comment on lines +93 to 94
df = df.map(ExportManager._sanitize_string)
df.to_excel(writer, sheet_name=sheet_name, index=False, startrow=start_row)
Copy link

Copilot AI Apr 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of df.map is intended to apply the sanitation function to every cell in the DataFrame, but DataFrame.map operates on columns (or fails entirely) while DataFrame.applymap should be used for elementwise operations. Consider replacing it with df = df.applymap(ExportManager._sanitize_string).

Suggested change
df = df.map(ExportManager._sanitize_string)
df.to_excel(writer, sheet_name=sheet_name, index=False, startrow=start_row)
df = df.applymap(ExportManager._sanitize_string)

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FutureWarning: DataFrame.applymap has been deprecated. Use DataFrame.map instead.

In Pandas versions prior to 2.1.0, DataFrame.applymap() was the correct method to apply an elementwise operation across a DataFrame.
However, since Pandas 2.1.0, applymap() has been deprecated and the recommended approach is to use DataFrame.map() for elementwise operations.
Therefore, if targeting Pandas 2.1.0 or newer, it is correct to replace applymap() with map().

@ImMin5 ImMin5 merged commit 1ff861f into cloudforet-io:master Apr 29, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request pass/signedoff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants