Skip to content

Conversation

@veer66
Copy link

@veer66 veer66 commented May 22, 2021

What does this changes

The O-ANG rule was convert into dev branch style.

What was wrong

update-royin and dev are conflicted.

How this fixes it

Write the rule in dev branch style

Your checklist for this pull request

🚨Please review the guidelines for contributing to this repository.

  • Passed code styles and structures
  • Passed code linting checks and unit test

@pep8speaks
Copy link

pep8speaks commented May 22, 2021

Hello @veer66! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 175:80: E501 line too long (86 > 79 characters)

Comment last updated at 2021-06-03 08:57:07 UTC

@veer66 veer66 mentioned this pull request May 29, 2021
2 tasks
@wannaphong wannaphong requested a review from bact June 1, 2021 08:56
@bact bact added the enhancement enhance functionalities label Jun 1, 2021
@bact bact added this to the 2.4 milestone Jun 1, 2021
Copy link
Member

@bact bact left a comment

Choose a reason for hiding this comment

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

One test failed.

FAIL: test_romanize_royin_basic (tests.test_transliterate.TestTransliteratePackage)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\projects\pythainlp-9y1ch\tests\test_transliterate.py", line 63, in test_romanize_royin_basic
    self.assertEqual(romanize(word, engine="royin"), expect)
AssertionError: 'hyak' != 'yak'
- hyak
? -
+ yak

@veer66
Copy link
Author

veer66 commented Jun 3, 2021

One test failed.

FAIL: test_romanize_royin_basic (tests.test_transliterate.TestTransliteratePackage)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\projects\pythainlp-9y1ch\tests\test_transliterate.py", line 63, in test_romanize_royin_basic
    self.assertEqual(romanize(word, engine="royin"), expect)
AssertionError: 'hyak' != 'yak'
- hyak
? -
+ yak

It is fixed by the latest commit.

@bact
Copy link
Member

bact commented Jun 3, 2021

The test that failed is from unrelated ConceptNet part, which we will investigate separately from this PR.

I think we are good to merge this for now.

@bact bact merged commit 26b5e8f into PyThaiNLP:update-royin Jun 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement enhance functionalities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants