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

Implement num_to_laoword #17

Merged
merged 2 commits into from
May 23, 2024
Merged

Conversation

gunsodo
Copy link
Contributor

@gunsodo gunsodo commented May 19, 2024

Solves #9.

Implemented num_to_laoword by using PyThaiNLP-style. Decided to rename the function from number2lao to num_to_laoword to maintain the consistency with other APIs within the same module. Test cases added to the suite.

Please let me know if there requires any fix. I myself is not a Lao-language user and was trying to refer from this Youtube lesson and this repo so I might have missed a few points.

Copy link
Owner

@wannaphong wannaphong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@gunsodo
Copy link
Contributor Author

gunsodo commented May 22, 2024

Noticed regression tests failed. Do you want me to fix this PR or open a new one?

@wannaphong
Copy link
Owner

Noticed regression tests failed. Do you want me to fix this PR or open a new one?

Can you fix in this PR?

@gunsodo
Copy link
Contributor Author

gunsodo commented May 23, 2024

@wannaphong coverage test seems to pass in my environment (both 3.8 and 3.9). Root cause seems to come from gensim (piskvorky/gensim#3525). Decided to use 1.10.1 to avoid deprecation warnings.

Still, there is a high chance the CI checks will still fail because of the coveralls version (as seen here, something similar to TheKevJames/coveralls-python#434 I assume).

Let me know if you want me to cap the coveralls version as well.

@gunsodo gunsodo requested a review from wannaphong May 23, 2024 12:41
@wannaphong wannaphong merged commit 8738c50 into wannaphong:master May 23, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants