Fix to_finals dropping the 'n' from syllabic 'ng' (嗯/呣/唔) in non-strict mode#371
Fix to_finals dropping the 'n' from syllabic 'ng' (嗯/呣/唔) in non-strict mode#371gaoflow wants to merge 1 commit into
Conversation
In non-strict mode get_finals() splits the syllabic velar nasal 'ng'
(readings of 嗯/兀/唔, e.g. ńg/ňg/ǹg) into initial 'n' + final 'g', so it
returns the invalid final 'g' and silently drops the 'n'. The existing
m/n guard only fires when the remaining final is empty, which never
happens for 'ng' ('g' is left over). Return the whole 'ng' as the final,
matching the sibling bare m/n behaviour. Strict mode (no final, by
design) is left unchanged.
📝 WalkthroughWalkthroughIn ng Finals Non-strict Fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pypinyin/style/_utils.py`:
- Around line 63-64: The new comment in the helper around pinyin splitting uses
fullwidth punctuation that triggers Ruff RUF003; update the comment in the
_utils.py logic near the handling of “ńg/嗯” to use ASCII punctuation only,
replacing the fullwidth parentheses and comma with standard ASCII equivalents
while keeping the meaning unchanged.
In `@tests/contrib/test_tone_convert.py`:
- Around line 292-293: Normalize the added test comment punctuation in
test_tone_convert so Ruff no longer flags RUF003; update the comment text around
the nasal-syllable note and the later note at the referenced spot to use ASCII
punctuation instead of fullwidth marks, keeping the same meaning while matching
the project's linting rules.
In `@tests/test_others.py`:
- Around line 77-78: Normalize the new test comments in test_others.py to use
ASCII punctuation only, since Ruff flags the fullwidth characters as RUF003.
Update the affected comment text around the assertions in the test module so the
same meaning remains but all punctuation is ASCII, and check the nearby comment
referenced by the same review to keep the file lint-clean.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 71993d7d-13c9-47d9-b8fd-0bd8358cc3a3
📒 Files selected for processing (3)
pypinyin/style/_utils.pytests/contrib/test_tone_convert.pytests/test_others.py
| # ńg（嗯 等）会被切出声母 n，剩下的 'g' 不是有效韵母， | ||
| # 非严格模式下应将整个 ng 作为韵母返回。 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the fullwidth punctuation in this new comment.
Lines 63-64 use fullwidth （, ）, and ，, which Ruff reports as RUF003. Please normalize them to ASCII so this change stays lint-clean.
🧰 Tools
🪛 Ruff (0.15.20)
[warning] 63-63: Comment contains ambiguous （ (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF003)
[warning] 63-63: Comment contains ambiguous ） (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(RUF003)
[warning] 63-63: Comment contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
[warning] 63-63: Comment contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pypinyin/style/_utils.py` around lines 63 - 64, The new comment in the helper
around pinyin splitting uses fullwidth punctuation that triggers Ruff RUF003;
update the comment in the _utils.py logic near the handling of “ńg/嗯” to use
ASCII punctuation only, replacing the fullwidth parentheses and comma with
standard ASCII equivalents while keeping the meaning unchanged.
Source: Linters/SAST tools
| # 嗯/呣/唔 等鼻音节：非严格模式下整个 m/n/ng 作为韵母， | ||
| # 严格模式下没有韵母（保持原有设计）。 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Normalize the punctuation in the added test comments.
Lines 292-293 and Line 300 contain fullwidth punctuation that Ruff flags as RUF003. Swapping those marks to ASCII avoids a needless lint failure here.
Also applies to: 300-300
🧰 Tools
🪛 Ruff (0.15.20)
[warning] 292-292: Comment contains ambiguous ： (FULLWIDTH COLON). Did you mean : (COLON)?
(RUF003)
[warning] 292-292: Comment contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
[warning] 293-293: Comment contains ambiguous （ (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF003)
[warning] 293-293: Comment contains ambiguous ） (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(RUF003)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/contrib/test_tone_convert.py` around lines 292 - 293, Normalize the
added test comment punctuation in test_tone_convert so Ruff no longer flags
RUF003; update the comment text around the nasal-syllable note and the later
note at the referenced spot to use ASCII punctuation instead of fullwidth marks,
keeping the same meaning while matching the project's linting rules.
Source: Linters/SAST tools
| # 嗯 等字的 ńg/ňg/ǹg 读音在非严格模式下，韵母应是完整的 ``ng``， | ||
| # 而不是把声母 n 切掉后剩下的无效韵母 ``g``。 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use ASCII punctuation in these new comments.
Lines 77-78 and Line 88 introduce fullwidth punctuation that Ruff reports as RUF003. Please normalize those characters so the test file remains lint-clean.
Also applies to: 88-88
🧰 Tools
🪛 Ruff (0.15.20)
[warning] 77-77: Comment contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
[warning] 77-77: Comment contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_others.py` around lines 77 - 78, Normalize the new test comments
in test_others.py to use ASCII punctuation only, since Ruff flags the fullwidth
characters as RUF003. Update the affected comment text around the assertions in
the test module so the same meaning remains but all punctuation is ASCII, and
check the nearby comment referenced by the same review to keep the file
lint-clean.
Source: Linters/SAST tools
1 participant
简述：非严格模式下，
to_finals会把鼻音节ng（嗯/兀/唔 等读音 ńg/ňg/ǹg）切成声母n+ 韵母g，丢掉了n，返回了不存在的韵母g。本 PR 让整个ng作为韵母返回，与单独的m/n保持一致；严格模式（无韵母）行为不变。The bug
In non-strict mode,
to_finalsdrops thenfrom the syllabic velar-nasal readingngand returns the invalid final'g':It propagates to every style and to the high-level API:
Why
'ng'is the correct resultto_finals('ń', strict=False) == 'n'andto_finals('ḿ', strict=False) == 'm'(the existing# ń, ḿguard returns the whole syllable).ngshould follow suit.'g'is not a valid Mandarin final at all (not in_FINALS), and thenphoneme is silently lost.''by design ([嗯]字的声母韵母问题 #284, m,n,ng 这几个拼音 strict=True 时作为既没有声母也没有韵母的情况处理？ #80:n/ng不是韵母) — that behaviour is unchanged.Root cause
pypinyin/style/_utils.py::get_finals: forng,get_initials('ng', strict=False)greedily matches the valid initialn, sofinals = 'ng'[len('n'):] = 'g'. The existing guardif not finals and not strict: return pinyinonly rescues the barem/n(where the remainder is empty); forngthe remainder'g'is non-empty, so the guard is skipped and the invalid'g'is returned. The fix adds thengcase (returns the whole syllable in non-strict mode), leaving the strict path and the general empty-final path untouched.Tests
tests/contrib/test_tone_convert.py: addedngcases (ńg/ňg/ǹg/ng) to theto_finals,to_finals_tone,to_finals_tone2,to_finals_tone3parametrize blocks — includingstrict=True → ''controls and theń → n/ḿ → msibling controls.tests/test_others.py::test_finals_syllabic_ng_keeps_ncovers the high-levelpinyin('嗯', …, heteronym=True, strict=False)blast radius and asserts the strict-default[['']]is preserved.The added positive cases fail on
master(assert 'g' == 'ng') and pass with the fix; full suite stays green (the only failures here are the pre-existingtests/test_cmd.pypty/OSErrorones, unrelated to this change).Summary by CodeRabbit
ngis now preserved as a whole final in non-strict mode.m,n, andngforms.嗯so non-strict conversions keepng-based finals, while strict mode remains unchanged.