Replace string literals with a call to format() - part of #5.

[cynthia]

This only fixes parts of #5 .

This fixes the Py3.6-isms, but will not fix cases where a string is expected to be unicode. I haven't checked if that is the case, but judging from the nature of the project the likelihood of a follow-up patch to fix that seems pretty high.

The tradeoff is that the syntax is verbose, but this ensures compatibility towards a larger audience.

[cynthia]

a0cfeb3 has two mistakes in the patch. Follow-up commit on the way.

[krikit]

Oops, We are not ready to get pull request. ;)

Let's divide the cases to use khaiii into two.

First

is to compile source code and to build resources. In this case, an isolated building environment will not effect other services. Yes, we respect others to use another version of python and PyTorch. That is why people use environment(or version) management systems, such as pyenv and virtualenv.

So why we use such high version of python? It is what we strongly intended for more simple and easy to maintain code. We have not enough time to test on various versions. Frankly, we could not spend all our efforts only on khaiii.

For this reason, I think the files under rsc directory are not good to be merged now. Let's talk more about this.

The second

case is python binding, the khaiii.py file. The API binding is possibly used at various python versions. So I think it is good to resolve f-strings literals to support python versions lower than 3.6. But we still not sure to support python version 2.

One more thing

Before you request merge, can you please run and fix lint errors? We use pylint and we recommend to fix all errors except 'R' level. khaiii.py you requested has some errors like below,

$ pylint khaiii.py
No config file found, using default configuration
************* Module khaiii
R: 27, 0: Too few public methods (0/2) (too-few-public-methods)
R: 44, 0: Too few public methods (0/2) (too-few-public-methods)
R: 67, 0: Too few public methods (1/2) (too-few-public-methods)
W:110, 8: Unused variable 'morphs_str' (unused-variable)
R: 98, 0: Too few public methods (1/2) (too-few-public-methods)
W:160,12: Unused variable 'ext' (unused-variable)

------------------------------------------------------------------
Your code has been rated at 9.69/10 (previous run: 9.75/10, -0.06)

Can you resolve above two 'W' level messages? If they are false alarms, you can suppress them by adding suppressing comment.

And the last thing,

(Sorry so many.. I know) Please fix on develop branch and give us pull requests also on develop branch. We are very sorry not to be ready, khaiii is just borned. ;) We will soon prepare a guide how to pull request from fork.

혹시.. 한국어가 가능하시다면 한국어로 대화하는건 어떠실런지요? 영어 실력이 미천하여 글을 쓰는데 많은 시간이 필요합니다. ㅠ.ㅠ

[cynthia]

So why we use such high version of python? It is what we strongly intended for more simple and easy to maintain code. We have not enough time to test on various versions. Frankly, we could not spend all our efforts only on khaiii.

For this reason, I think the files under rsc directory are not good to be merged now. Let's talk more about this.

어디까지나 외부에서 사용하는 사람 입장에서의 제안이니 참고 사항으로 봐주셨으면 좋겠습니다.

오픈소스 라이브러리가 성공하기 위해서는 사용할 수 있는 사람이 많아야 합니다. 현재 파이썬 3.7의 경우는 거의 시장 보급이 안된 상태이고, 3.6도 그렇게 많이 사용하고 있지 않은게 현실입니다. 특히나, 현 라이브러리 와 같이 자연어 처리 연구에 도움이 되는 경우는 연구실같은 환경 특성상 구성원들이 라이브러리 업데이트를 잘 하지 않습니다. (대부분은 논문 데드라인 쫓아가기 바빠서...)

이런것을 감안해서, 모처럼 많은 노력을 들여 만든 훌륭한 라이브러리를 여러 사람이 사용할 수 있도록 커버러지를 늘렸으면 좋겠다는 바램입니다. 당장 빌드를 함에 있어서 이슈 트래커를 보니 사용을 못하시는 분들이 있다는 점에서 필요성을 느꼈습니다.

다중 파이썬 버전의 경우, 차후 Travis CI 같은걸 연동함으로서 해결이 가능할 것으로 생각됩니다. 시간적으로 제약이 있다는 점, 그리고 귀사에서 다른 버전에 대한 지원 우선 순위가 낮다는 점은 충분히 이해합니다. 목마른 사람이 우물 파는게 오픈소스의 시장 경제 원리이기도 하니까요. 당장 저희 회사 환경에서는 3.5를 아직 사용하는 양산 환경들이 있어, 혹 사용하게 될 경우 3.5에 대한 지원이 필요했습니다. 그래서 패치를 보내드린 것이고, 머지를 할지 말지는 어디까지나 프로젝트 오너가 판단할 몫입니다.

(철학적으로 추구하는 바가 다를 수 있기 때문에, 그 불완전성을 채워주기 위해서 fork라는것도 존재하는 것이구요. 실례로 귀사의 라이브러리 중 N2도 폐사에서 OpenMP / Intel Compiler 지원 문제로 fork를 하였습니다.)

case is python binding, the khaiii.py file. The API binding is possibly used at various python versions. So I think it is good to resolve f-strings literals to support python versions lower than 3.6. But we still not sure to support python version 2.

이 점에 대해서는 저도 공수 측면에서는 공감합니다. 그래서 우선 3.x 대의 호환성을 먼저 보내드린 것이구요, 2.x 대는 상기 PR의 패치로 아마도 해결되지 않을것으로 보입니다.

Before you request merge, can you please run and fix lint errors? We use pylint and we recommend to fix all errors except 'R' level. khaiii.py you requested has some errors like below,

pylint만 통과하면 문제가 없는건지요? 이상하게도 format() 안에서 참조하는 변수의 경우, pylint에서 locals()로 간접 전달이 일어날 수 있다는걸 캐치하지 못하는것 같습니다. 깔끔하고 안전하게 잡는 방법은, 필요한 변수만 format에 넘겨주는 방법이 있을듯 합니다만, 이 경우에는 이후에 뭔가 더 추가 될 경우 불편하다는 단점이 있습니다. 코드 자체를 보았을때는 그런 일은 없을 듯 합니다만, 우선 방향만 알려주시면 fixup 커밋 보내겠습니다.

개인적으로는 필요한 것만 format으로 넘기든지, '%s' % var 형태로도 클로져내 참조를 하는 방법이 가장 나을듯 합니다.

(Sorry so many.. I know) Please fix on develop branch and give us pull requests also on develop branch. We are very sorry not to be ready, khaiii is just borned. ;) We will soon prepare a guide how to pull request from fork.

develop 브랜치에 깔끔하게 머지가 안된다면 리베이스 해드리겠습니다.

[cynthia]

두 PR 다 확인해본 결과 develop 브랜치에 깨끗하게 머지되니, master에 제 remote에서 pull 해가시고 develop에 랜딩 한 다음에 PR 티켓은 닫으셔도 될것 같습니다.