Replace mecab-python3 with fugashi for Japanese tokenization #6086
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This replaces mecab-python3 with fugashi for Japanese tokenization. I am the maintainer of both projects. Both projects are MeCab wrappers, so the underlying C++ code is the same. fugashi is the newer wrapper and doesn't use SWIG, so for basic use of the MeCab API it's easier to use. This code insures the use of a version of ipadic installed via pip, which should make versioning and tracking down issues easier. fugashi has wheels for Windows, OSX, and Linux, which will help with issues with installing old versions of mecab-python3 on Windows. Compared to mecab-python3, because fugashi doesn't use SWIG, it doesn't require a C++ runtime to be installed on Windows. In adding this change I removed some code dealing with `cursor`, `token_start`, and `token_end` variables. These variables didn't seem to be used for anything, it is unclear to me why they were there. I ran the tests and they passed, though I couldn't figure out how to run the slow tests (`--runslow` gave an error) and didn't try testing with Tensorflow.
polm
changed the title
Forgot to delete this...
Codecov Report
@@ Coverage Diff @@
## master #6086 +/- ##
==========================================
+ Coverage 78.35% 79.50% +1.14%
==========================================
Files 146 146
Lines 26454 26450 -4
==========================================
+ Hits 20729 21030 +301
+ Misses 5725 5420 -305
Continue to review full report at Codecov.
|
JetRunner
approved these changes
Jul 28, 2020
There was a problem hiding this comment.
LGTM. As long as the tokenization results are the same (it should according to your note).
Maybe @LysandreJik wants to make sure the dependency is good?
sgugger
approved these changes
Jul 29, 2020
There was a problem hiding this comment.
LGTM as well and since the tests pass, the tokenization looks the same. Thanks a lot for tackling this!
Only place the mecab github is linked is in pretrained_models.rst from a quick search. Maybe we can add the change of link to this PR to make it perfectly clean? I can force-push on your branch if you prefer me to do it.
|
My pleasure. It'd be great if you could edit the docs 👍 I think I gave edit permissions so you should be able to push normally. |
sgugger
reviewed
Jul 29, 2020
setup.py
Outdated
| @@ -97,7 +97,7 @@ | |||
| "isort @ git+git://github.com/timothycrosley/isort.git@e63ae06ec7d70b06df9e528357650281a3d3ec22#egg=isort", | |||
| "flake8", | |||
| ] | |||
| extras["dev"] = extras["testing"] + extras["quality"] + ["mecab-python3<1", "scikit-learn", "tensorflow<=2.2", "torch"] | |||
| extras["dev"] = extras["testing"] + extras["quality"] + ["fugashi>=1.0,<2.0", "ipadic", "scikit-learn", "tensorflow<=2.2", "torch"] | |||
There was a problem hiding this comment.
Side note: the deps here are not with the same version tags as the deps up there.
LysandreJik
approved these changes
Jul 31, 2020
|
Awesome, this has been a small but sore thorn in some issues posted here. Thanks a lot @polm! |
|
If you ever have any other issues with it please feel free to tag me any time. |
Mehrad0711
pushed a commit
to Mehrad0711/transformers
that referenced
this pull request
Aug 3, 2020
…face#6086) * Replace mecab-python3 with fugashi This replaces mecab-python3 with fugashi for Japanese tokenization. I am the maintainer of both projects. Both projects are MeCab wrappers, so the underlying C++ code is the same. fugashi is the newer wrapper and doesn't use SWIG, so for basic use of the MeCab API it's easier to use. This code insures the use of a version of ipadic installed via pip, which should make versioning and tracking down issues easier. fugashi has wheels for Windows, OSX, and Linux, which will help with issues with installing old versions of mecab-python3 on Windows. Compared to mecab-python3, because fugashi doesn't use SWIG, it doesn't require a C++ runtime to be installed on Windows. In adding this change I removed some code dealing with `cursor`, `token_start`, and `token_end` variables. These variables didn't seem to be used for anything, it is unclear to me why they were there. I ran the tests and they passed, though I couldn't figure out how to run the slow tests (`--runslow` gave an error) and didn't try testing with Tensorflow. * Style fix * Remove unused variable Forgot to delete this... * Adapt doc with install instructions * Fix typo Co-authored-by: sgugger <sylvain.gugger@gmail.com> Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This replaces mecab-python3 with fugashi for Japanese tokenization. I am
the maintainer of both projects.
Both projects are MeCab wrappers, so the underlying C++ code is the
same. fugashi is the newer wrapper and doesn't use SWIG, so for basic
use of the MeCab API it's easier to use.
This code insures the use of a version of ipadic installed via pip,
which should make versioning and tracking down issues easier.
fugashi has wheels for Windows, OSX, and Linux, which will help with
issues with installing old versions of mecab-python3 on Windows.
Compared to mecab-python3, because fugashi doesn't use SWIG, it doesn't
require a C++ runtime to be installed on Windows.
In adding this change I removed some code dealing with
cursor,token_start, andtoken_endvariables. These variables didn't seem tobe used for anything, it is unclear to me why they were there.
I ran the tests and they passed. For reference, since I had trouble figuring it out, this is needed to run the tests:
This is a followup to #5375 .
It's not in this PR, but because installing MeCab separately is not required, it might be a good idea to have the docs changed to point directly to fugashi instead of MeCab.