Fix kuromoji default stoptags#26600
Merged
cbuescher merged 2 commits intoelastic:masterfrom Sep 15, 2017
Merged
Conversation
The order was reversed, as the expected value was given for the actual value and vice versa. This led to a confusing assertion error message: ``` FAILURE 0.04s J1 | KuromojiAnalysisTests.testPartOfSpeechFilter <<< FAILURES! > Throwable elastic#1: java.lang.AssertionError: expected different term at index 1 > Expected: "が" > but: was "おいしい" ``` when the string "が" was actually not expected.
* add new test which checks that part-of-speech tokens are removed when using the kuromoji_part_of_speech filter * initialize the default stop-tags in `KuromojiPartOfSpeechFilterFactory` if the `stoptags` are not given in the config
Collaborator
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
Member
|
@elasticmachine test this please |
Member
|
@johtani looks like you might be the right person to look at this. Please un-assign yourself if not. |
johtani
approved these changes
Sep 15, 2017
Contributor
johtani
left a comment
There was a problem hiding this comment.
LGTM
@cbuescher This PR is bug fix so we can cherry-pick it to 6.x, right?
Member
|
@johtani if it is a bug fix an doesn't change existing behaviour I'd even pick it to 6.0. Looks low risk to me, do you agree? |
Contributor
|
@cbuescher I agree with you. |
Member
|
@avdv thanks a lot for this fix, I will merge this to master and the current 6.x branches |
cbuescher
pushed a commit
that referenced
this pull request
Sep 15, 2017
Initialize the default stop-tags in `KuromojiPartOfSpeechFilterFactory` if the `stoptags` are not given in the config. Also adding a test which checks that part-of-speech tokens are removed when using the kuromoji_part_of_speech filter.
cbuescher
pushed a commit
that referenced
this pull request
Sep 15, 2017
Initialize the default stop-tags in `KuromojiPartOfSpeechFilterFactory` if the `stoptags` are not given in the config. Also adding a test which checks that part-of-speech tokens are removed when using the kuromoji_part_of_speech filter.
Contributor
Author
|
thank for @cbuescher and @johtani! any chance to get this into 5.6.x, too? |
Member
|
@avdv you are right, since this is a bugfix, I think we should also merge to the 5.6 branch. Will do so. Not sure though in which of the next minor releases it is going to end up. |
cbuescher
pushed a commit
that referenced
this pull request
Sep 15, 2017
Initialize the default stop-tags in `KuromojiPartOfSpeechFilterFactory` if the `stoptags` are not given in the config. Also adding a test which checks that part-of-speech tokens are removed when using the kuromoji_part_of_speech filter.
jasontedor
added a commit
to jasontedor/elasticsearch
that referenced
this pull request
Sep 16, 2017
* master: fix testSniffNodes to use the new error message Add check for invalid index in WildcardExpressionResolver (elastic#26409) Docs: Use single-node discovery.type for dev example Filter unsupported relation for range query builder (elastic#26620) Fix kuromoji default stoptags (elastic#26600) [Docs] Add description for missing fields in Reindex/Update/Delete By Query (elastic#26618) [Docs] Update ingest.asciidoc (elastic#26599) Better message text for ResponseException [DOCS] Remove edit link from ML node enable bwc testing fix StartRecoveryRequestTests.testSerialization Add bad_request to the rest-api-spec catch params (elastic#26539) Introduce a History UUID as a requirement for ops based recovery (elastic#26577) Add missing catch arguments to the rest api spec (elastic#26536)
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Fixes #26519