Added Japanese language support - #278
Conversation
|
This looks great, I'm about to merge and accept but can you just add one unit test where 'ja' language works? |
|
I've added a unit test for 'ja' language as you requested and replaced |
|
Any progress on this front? Adding ja support would be great! |
|
@Newspad So Japanese text extraction is only supported on CentOS? If I don't use |
codelucas
left a comment
There was a problem hiding this comment.
Hey @Newspad thanks for the great work on this 💯 sorry I've been busy and not been able to look for so long. I will take a look (and potentially add patch commits to your PR if there are merge conflicts.
cc @Evilplanet
| 頃 | ||
| 類 | ||
| 首 | ||
| 高 No newline at end of file |
There was a problem hiding this comment.
So many stopwords! 416? Does this seem right, I did some googling and found other Japanese stopwords files to be ~120 in length. e.g. https://github.com/stopwords-iso/stopwords-ja/blob/master/stopwords-ja.txt
| words.append(word) | ||
| node = node.next | ||
| except Exception as e: | ||
| pass |
There was a problem hiding this comment.
instead of pass and we please add exception logging in a pretty format saying Japanese stopwords extraction has failed. (in your experience does candidate_words have a high rate of failure? If not I'd even suggest no exception catching here.
| words = [] | ||
| try: | ||
| import MeCab | ||
| import os, subprocess |
There was a problem hiding this comment.
Can we have a separate try/catch for these two import statements and print a pretty "you must install mecab .. etc" if it is not installed?
| $ curl https://raw.githubusercontent.com/codelucas/newspaper/master/download_corpora.py | python3 | ||
|
|
||
|
|
||
| **If you are on CentOS** and use Japanese language support, install using the following: |
There was a problem hiding this comment.
Not clear if this means:
- Japanese language support works by default or all non-CentOS operating systems and you don't need to install
- Japanese language support works ONLY on CentOS, and if you are on CentOS install.
If 2) is true, I think this design is not ideal. A language support should be for all operating systems, not just one because a parse/tokenizing library is available only on that OS.
|
For AWS Lambda anyway we will put all dependencies into deployment zip - so MeCab will be there too. |
|
Hi Any updates on JP support ? |
|
@asafcombo The problem with Japanese support is you have to install MeCab, which is not easy feat. It really depends on your environment. You could take Newspad's code and make sure MeCab is installed on your server. Change the code where MeCab is called if necessary. (I use natto-py to call MeCab) |
|
#584 is merged! Giving us JP support. Thanks again for this initial effort here @Newspad but I decided to merge PR-584 as it has minimal dependency requirements. Also unfortunately we don't have a good way to measure branch vs. branch performance in article extraction quality (will file a todo for myself) |
I've added Japanese language support and it works fine.
This change doesn't affect any other code.