A possibly unreachable branch in royin.py

[veer66]

In royin.py:

I suppose the last else block is unreachable since word[i] == _RO_RUA should be always true in this whole block.

elif word[i] == _RO_RUA and i + 1 < len(word):
    if word[i + 1] == _RO_RUA:
        word = list(word)
        del word[i + 1]
        if i + 2 == len_cons:
            word[i] = "an"
        else:
            word[i] = "a"
        word = "".join(word)
        i += 1
    elif word[i] == _RO_RUA:
        word = word.replace(
            consonants[i], _CONSONANTS[consonants[i]][1]
        )
        i += 1
    else:
        word = word.replace(
            consonants[i],
            _CONSONANTS[consonants[i]][1]
        )
        i += 1

[wannaphong]

Can you fork then send to update-royin branche?

[veer66]

@wannaphong I failed to run tests. I use Python 3.9.4 on Fedora 34. I have already run "pip install -r requirements.txt". I run tests by python -m unittest discover -s tests -p 'test_transliterate.py'.

However, I need to install following pakcages.

pip install torch
pip install epitran
pip install pyicu

Should I add them to requirements.txt?

[wannaphong]

@wannaphong I failed to run tests. I use Python 3.9.4 on Fedora 34. I have already run "pip install -r requirements.txt". I run tests by python -m unittest discover -s tests -p 'test_transliterate.py'.

However, I need to install following pakcages.

pip install torch
pip install epitran
pip install pyicu

Should I add them to requirements.txt?

You can use pip install -e .[full] for install all requirements.

[veer66]

I ran python -m unittest discover -s tests -p 'test_transliterate.py' on commit 3ea5f5e. It didn't pass two tests.

Did I do it properly, for example, correct commit/branch? Should I just ignore these two tests for now?

..FF...
======================================================================
FAIL: test_romanize_royin_basic (test_transliterate.TestTransliteratePackage)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vee/Develop/free/pythainlp/tests/test_transliterate.py", line 63, in test_romanize_royin_basic
    self.assertEqual(romanize(word, engine="royin"), expect)
AssertionError: 'nppn' != 'nopphon'
- nppn
+ nopphon
?  +  ++


======================================================================
FAIL: test_romanize_royin_consistency (test_transliterate.TestTransliteratePackage)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vee/Develop/free/pythainlp/tests/test_transliterate.py", line 67, in test_romanize_royin_consistency
    self.assertEqual(
AssertionError: 'takpai' != 'takbai'
- takpai
?    ^
+ takbai
?    ^


----------------------------------------------------------------------
Ran 7 tests in 0.350s

FAILED (failures=2)

[wannaphong]

I ran python -m unittest discover -s tests -p 'test_transliterate.py' on commit 3ea5f5e. It didn't pass two tests.

Did I do it properly, for example, correct commit/branch? Should I just ignore these two tests for now?

..FF...
======================================================================
FAIL: test_romanize_royin_basic (test_transliterate.TestTransliteratePackage)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vee/Develop/free/pythainlp/tests/test_transliterate.py", line 63, in test_romanize_royin_basic
    self.assertEqual(romanize(word, engine="royin"), expect)
AssertionError: 'nppn' != 'nopphon'
- nppn
+ nopphon
?  +  ++


======================================================================
FAIL: test_romanize_royin_consistency (test_transliterate.TestTransliteratePackage)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vee/Develop/free/pythainlp/tests/test_transliterate.py", line 67, in test_romanize_royin_consistency
    self.assertEqual(
AssertionError: 'takpai' != 'takbai'
- takpai
?    ^
+ takbai
?    ^


----------------------------------------------------------------------
Ran 7 tests in 0.350s

FAILED (failures=2)

OK. I fixed.

[bact]

I guess this meant to handle รร
but I'm not sure about the algorithm itself.

Any clarification on the algo?

[bact]

See #415 and #553

[veer66]

I suggest to refactor the function _replace_consonants first. I did it on the Ruby port. I probably try to translate it back to Python. Of course, I won't try to use the reduce function in Python. What I did was elimilating embedded if blocks and duplicated blocks.

def replace_consonants(word, consonants)
  return word unless consonants
  return word.gsub(consonants[0], CONSONANTS[consonants[0]][0]) if consonants.length == 1
  consonants.reduce({rom: "", th: word}) do |w, consonant|
    non_thai = w[:th].match(/^[^ก-์]+/)
    if non_thai
      w[:rom] += non_thai.to_s
      w[:th] = w[:th][non_thai.to_s.length..-1]
    end
    if w[:skip]
      {rom: w[:rom], th: w[:th]}
    elsif w[:rom] == "" and w[:th] == "ห"
      {rom: "", th: w[:th][1..-1]}
    elsif w[:rom] == ""
      {rom: CONSONANTS[consonant][0], th: w[:th][consonant.length..-1]}
    elsif consonant == "ร" and w[:th] == "รร"
      {rom: w[:rom] + "an", th: w[:th][2..-1], skip: true}
    elsif consonant == "ร" and w[:th][0..1] == "รร"
      {rom: w[:rom] + "a", th: w[:th][2..-1], skip: true}
    else
      {rom: w[:rom] + CONSONANTS[consonant][1], th: w[:th][consonant.length..-1]}
    end
  end[:rom]
end

[veer66]

After refactoring, adding new rules to handle cases in #415 will be much more convenient.

[veer66]

I guess this meant to handle รร

No, the deleted block has nothing to do with รร. รร is handled by another block.

elif word[i] == _RO_RUA and i + 1 < len(word):
    if word[i + 1] == _RO_RUA:
        word = list(word)
        del word[i + 1]
        if i + 2 == len_cons:
            word[i] = "an"
        else:
            word[i] = "a"
        word = "".join(word)
        i += 1

but I'm not sure about the algorithm itself.

Any clarification on the algo?