Skip to content

fix: handle empty string with errors='replace'#372

Open
santhreal wants to merge 1 commit into
mozillazg:masterfrom
santhreal:fix/replace-empty-string
Open

fix: handle empty string with errors='replace'#372
santhreal wants to merge 1 commit into
mozillazg:masterfrom
santhreal:fix/replace-empty-string

Conversation

@santhreal

@santhreal santhreal commented Jul 18, 2026

Copy link
Copy Markdown

converter._convert_nopinyin_chars hits TypeError when ord empty string errors=replace. This PR fixes the regression with a focused test covering the case.

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of empty text when using replacement or ignore modes.
    • Empty input now consistently returns an empty result without triggering errors.
  • Tests

    • Added coverage to verify expected behavior for empty input across supported error-handling modes.

pinyin('', errors='replace') called ord('') and raised TypeError.
Treat length 0 like the multi-char path so replace returns '' and the
public API yields [], matching default/ignore.
@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The converter treats empty replacement input like other non-single-character inputs, avoiding ord() conversion. Tests verify that empty strings return empty lists for default, replace, and ignore error modes.

Changes

Empty input handling

Layer / File(s) Summary
Handle and test empty input
pypinyin/converter.py, tests/test_pinyin.py
The replacement branch now includes empty input, and tests cover empty strings with replace and ignore error modes.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main fix: handling empty strings with errors='replace'.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant