MycroftAI / lingua-franca

Mycroft's multilingual text parsing and formatting library
Apache License 2.0
75 stars 79 forks source link

Implement function to tie percent sign to number #201

Closed Badboy-16 closed 2 years ago

Badboy-16 commented 3 years ago

Description

Fixes #196

Adds a function to tie the percent sign (%) to the preceding numbers.

Type of PR

Testing

Testing added in test_parse_common.py. (test_normalize())

ChanceNCounter commented 3 years ago

Sorry, I think I failed to communicate the core problem with #196.

The larger issue is that the normalizer is returning "X %.", instead of "X % .", because it doesn't generally insert a space before punctuation. Consumers looking for that percent sign in isolation miss it when it has punctuation attached to it.

The space between the number and the percent sign is apparently useful downstream, so we might have to revert that part.

Badboy-16 commented 3 years ago

It's ok @ChanceNCounter

So in that case, should the behaviour of normalizer be that it inserts a space between the percent sign and the punctuation? If so, I think I can revert my previous code and add a function to check if any percent signs preceding any punctuations, and if yes, insert a space between them.

ChanceNCounter commented 3 years ago

That's the desired behavior, yes. I dunno if the other discussion is closed forever =P but that's the part that should change for now. The trouble with changing behavior such as the separate percent sign is that existing code might be looking for that output.

We can make changes like that, with an appropriate deprecation period, but it's no small thing and the discussion has to brew for a while. Apparently our predecessors put that space in there on purpose.