fix(analyzer): register KrPassportRecognizer and align its default language#2170
Open
developer0hye wants to merge 2 commits into
Open
fix(analyzer): register KrPassportRecognizer and align its default language#2170developer0hye wants to merge 2 commits into
developer0hye wants to merge 2 commits into
Conversation
…nguage KrPassportRecognizer was the only Korean recognizer missing from default_recognizers.yaml. Its constructor did not accept the `name` keyword argument that the YAML loader passes to every predefined recognizer, so adding it to the YAML raised TypeError. Its default supported_language was also "kr" while every other Korean recognizer (RRN/BRN/FRN/driver license) defaults to "ko", so an AnalyzerEngine running "ko" silently skipped it. Add the `name` argument, change the default language to "ko", and register the recognizer in default_recognizers.yaml (enabled: false, country_code: kr) like its siblings. Add regression tests covering the default language, the `name` kwarg, and end-to-end loading from the YAML registry for both "ko" and "kr". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Change Description
KrPassportRecognizerwas the only Korean recognizer missing fromdefault_recognizers.yaml, and two inconsistencies with its siblings kept it out of the predefined registry:RecognizerListLoader.get) instantiates every predefined recognizer with anamekeyword argument, butKrPassportRecognizer.__init__did not acceptname. Adding the recognizer todefault_recognizers.yamltherefore raisedTypeError: __init__() got an unexpected keyword argument 'name'.KrRrnRecognizer,KrBrnRecognizer,KrFrnRecognizerandKrDriverLicenseRecognizerall already acceptname.supported_languagewaskr, while every other Korean recognizer defaults toko. fix(analyzer): update Korean language code from 'kr' to 'ko' #1742 already migrated the Korean recognizers fromkrtoko(correct ISO 639-1 code) and warned that the wrong code breaks loading once a Korean NLP model such as spaCy is configured.KrPassportRecognizerwas added later in feat: Add Korean passport number recognizer (KR_PASSPORT) #1814 and reintroducedkr, so anAnalyzerEnginerunningkosilently skips it.Before:
KrPassportRecognizeris absent fromdefault_recognizers.yaml, cannot be loaded through the registry (TypeErrorif listed), and does not participate in akoanalysis when instantiated with defaults.After: the constructor accepts
name, defaults toko, and is registered indefault_recognizers.yamlwithenabled: falseandcountry_code: kr(matching the other Korean recognizers per the country-specific recognizer guidelines). It now loads from the registry for bothkoandkronce enabled.Added regression tests covering the default language, the
namekeyword argument, and end-to-end loading from the YAML registry for bothkoandkr.Issue reference
No existing issue. Follows up on #1742 (Korean
kr->komigration) and #1814 (which added this recognizer).Checklist
🤖 Generated with Claude Code