Repository navigation
Fix Spanish number normalizer dropping the mil multiplier after a hundreds-block - #46
Open
selimfedakar wants to merge 1 commit into
Open
selimfedakar wants to merge 1 commit into
selimfedakar wants to merge 1 commit into
Conversation
_parse_number returned a hundreds-block (+ optional 0-99 remainder) immediately without checking for a following 'mil', so the thousands multiplier was dropped: 'doscientos cincuenta mil' normalized to '250 mil' instead of '250000'. Unify the hundreds value and apply the shared mil check (with a recursive tail parse). Adds a Spanish number normalizer test file (none existed); it fails on main for the mil cases.
This branch has not been deployed
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.
What
Fix a dropped thousands multiplier in the Spanish number normalizer (
normalization/languages/spanish/number_normalizer.py,_parse_number).A hundreds-block (
cien/ciento/doscientos…novecientos) optionally followed by a 0–99 remainder returned its value immediately and never checked whether the next word wasmil. So the thousands multiplier was silently dropped whenever a hundreds-block carried a sub-part beforemil. (Themillookahead existed only on the standalone 0–99 path and the bareciento milspecial case.)Fix
Unify the hundreds-block into a single value, then apply the shared
milmultiplier check (mirroring the existing 0–99 +millogic), with a recursive tail parse so a trailing remainder aftermilis added.Before / after (public API)
doscientos cincuenta mil euros250 mil euros250000 eurosciento cincuenta mil150 mil150000novecientos noventa y nueve mil999 mil999000doscientos cincuenta mil quinientos250 mil quinientos250500Non-regression:
doscientos cincuenta→250,cien→100,veintitres→23all unchanged.Reachability / tests
spanish/operators.pyexpand_written_numbers→SpanishNumberNormalizer(used bypresets/gladia-3.yaml); confirmed live viaload_pipeline("gladia-3", "es").normalize(...). Addstests/unit/languages/spanish_number_normalizer_test.py(the Spanish normalizer had no dedicated test file); it fails onmainfor themilcases and passes with this change. Full suite stays green.