Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Number spelling #682

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Number spelling #682

wants to merge 9 commits into from

Conversation

akx
Copy link
Member

@akx akx commented Dec 31, 2019

Thank you @blagasz for a great initial effort! I cleaned up things here and there (refactoring, performance issues, even some bugs).

There's also a smoke test to try and verify we get some result for every technically supported locale, but there are some infinite recursion cases involving year formatting I couldn't really figure out. These are marked XFAIL for the time being:

XFAIL tests/test_number_spelling.py::test_spelling_smoke[zh_Hant-year]
XFAIL tests/test_number_spelling.py::test_spelling_smoke[zh-year]
XFAIL tests/test_number_spelling.py::test_spelling_smoke[yue_Hans-year]
XFAIL tests/test_number_spelling.py::test_spelling_smoke[ja-year]
XFAIL tests/test_number_spelling.py::test_spelling_smoke[yue-year]
XFAIL tests/test_number_spelling.py::test_spelling_smoke[ak-year]

In addition, fractional formatting results are rather strange, but as the original PR's description said, it's still a work in progress.

Closes #660 (supersedes it)

@blagasz
Copy link
Member

blagasz commented Feb 23, 2020

Thank you @akx for your improvements!

I was trying to figure out the recursion but I could not reproduce it in my original code.

I also found an error regarding the default ruleset for the year=True flag. How should I commit not to mess up your concept?

I would also improve the smoke test by stepping through the available public rulesets instead of (None, 'year', 'ordinal') as the last two would never result in a valid ruleset (only year=True or ordinal=True would, but I messed up the default for year anyway).

@nagylzs
Copy link

nagylzs commented Jul 30, 2020

Hello! I have my own implementation for number spelling (Hungarian only), but I just found out that this may be part of babel. Looks like tests for unsupported python versions are failed only. How can I help to finish the implementation?

@blagasz
Copy link
Member

blagasz commented Jul 30, 2020

Hi, nice to have more people on board :)

I also started with my own implementation (#114) then the folks here convinced me that the CLDR RBNF is the way to go. I think the basic implementation of the RBNF rule engine is working now and @akx cleaned up the code.

I think we need to improve the tests and somehow reproduce the recursion error to address that, basic tests for English and Hungarian already run smoothly.

@akx akx force-pushed the number-spelling branch 2 times, most recently from 31815c9 to 9731112 Compare January 28, 2022 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants