Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add UniDic implimentation #218

Merged
merged 2 commits into from
Jun 20, 2023
Merged

Add UniDic implimentation #218

merged 2 commits into from
Jun 20, 2023

Conversation

mosuka
Copy link
Contributor

@mosuka mosuka commented Jun 17, 2023

Pull Request

What does this PR do?

  • Add UniDic implementation to allow consistent tokenization for searching and indexing.
  • Please see discussion comment

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

@ManyTheFish ManyTheFish self-requested a review June 19, 2023 07:47
Copy link
Member

@ManyTheFish ManyTheFish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @mosuka, thank you for your PR!
Because these two feature flags are incompatible, we should add a compiling error in case they are both activated:

#[cfg(all(feature = "japanese-segmentation-ipadic", feature = "japanese-segmentation-unidic"))]
compile_error!("Feature japanese-segmentation-ipadic and japanese-segmentation-unidic are mutually exclusive and cannot be enabled together");

@mosuka
Copy link
Contributor Author

mosuka commented Jun 19, 2023

@ManyTheFish
Thank you for your comment.
I'll fix it. 👍

@mosuka mosuka requested a review from ManyTheFish June 19, 2023 12:04
@mosuka
Copy link
Contributor Author

mosuka commented Jun 19, 2023

@ManyTheFish
I have modified it as you advised.
Please check it again.

Copy link
Member

@ManyTheFish ManyTheFish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @mosuka!

Bors merge

@meili-bors
Copy link
Contributor

meili-bors bot commented Jun 20, 2023

Build succeeded:

  • tests

@meili-bors meili-bors bot merged commit 366417d into meilisearch:main Jun 20, 2023
@meili-bot
Copy link
Contributor

This message is sent automatically

Thank you very much for submitting your PR! Did you know that throughout the month of June we’re holding a rafle?
If you share the link to your merged PR in our #giveaway Discord channel, you’ll automatically join a lottery for a chance at winning some Meiliswag. 🙂
Don’t hesitate to join us: https://discord.com/channels/1006923006964154428/1111273670657200198

@mosuka mosuka deleted the unidic branch June 21, 2023 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants