common: fix match_case misidentifying all-caps contractions as capitalized#4314
Open
nac7 wants to merge 2 commits into
Open
common: fix match_case misidentifying all-caps contractions as capitalized#4314nac7 wants to merge 2 commits into
nac7 wants to merge 2 commits into
Conversation
…lized match_case iterated over all characters including apostrophes when checking islower()/isupper(). Apostrophes return False for both islower() and isupper() in Python (they have no case), so words like "AIN'T" and "DON'T" would fail the all-caps check and fall through to the capitalize branch, producing "Is not" instead of "IS NOT". Fix: filter to only alphabetic characters before case-checking, so non-alpha chars in contractions no longer interfere with the detection. Affects ContractionPerturbation, ExpansionPerturbation, and SynonymPerturbation for all-caps inputs containing apostrophes. Fixes stanford-crfm#4313
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4313.
Problem
match_caseinsrc/helm/common/general.pyiterates over all characters when checkingislower()/isupper(), including apostrophes. In Python,"'".islower()and"'".isupper()both returnFalse(apostrophes have no case). This breaks the all-caps detection for any word containing an apostrophe:For
"AIN'T":all(ch.isupper() for ch in "AIN'T")isFalsebecause"'".isupper()isFalse, so the all-caps branch is skipped. The code falls into the capitalized branch (source_word[0].isupper()isTrue), producing"Is not"instead of"IS NOT".This affects
ContractionPerturbation,ExpansionPerturbation, andSynonymPerturbation, which all callmatch_caseto preserve source-word casing. All-caps contractions in benchmark inputs produce wrong-case augmented outputs.Fix
Filter to only alphabetic characters before case-checking. Non-alpha characters (apostrophes, digits, punctuation) are ignored when determining the case category of a word:
Tests
Added to
test_perturbation.py:test_match_case_all_caps_with_apostrophe—"AIN'T"/"DON'T"/"CAN'T"→"IS NOT"/"DO NOT"/"CANNOT"(regression cases)test_match_case_lowercase_with_apostrophe— lowercase contractions still produce lowercasetest_match_case_capitalized_with_apostrophe— capitalized contractions still capitalizetest_match_case_no_alpha_chars— empty / digit-only source → target unchangedtest_match_case_regular_words— regular words (no apostrophe) still work correctly