meilisearch / charabia

Library used by Meilisearch to tokenize queries and documents

MIT License

258 stars 89 forks source link

Adds a new normalizer to normalize œ to oe and æ to ae #278

Closed Soham1803 closed 5 months ago

Soham1803 commented 7 months ago

Pull Request

Related issue

Fixes #268

What does this PR do?

Creates a new normalizer ae_oe_normalizer
normalizes œ and Œ to oe, æ and Æ to ae.

PR checklist

Please check if your PR fulfills the following requirements:

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

Thank you so much for contributing to Meilisearch!

ManyTheFish commented 6 months ago

Hello @Soham1803, Thank you for your PR, I made several change requests before accepting to merge it,

let me know if you need more informations

Soham1803 commented 6 months ago

Hey @ManyTheFish I have made the requested changes can you please check and let me know. Thank You!

ManyTheFish commented 6 months ago

Hello @Soham1803,

The tests don't seem to work. Could you fix them?

Thank you

meili-bors[bot] commented 6 months ago

Build failed:

tests

Soham1803 commented 6 months ago

Hey @ManyTheFish, I'm trying to run the tests on my local machine but got some issues with jemalloc-sys in .cargo. The last commit I made is based on the logs of test run I read on Github. Its good if the tests pass now, or else I will make another commit after being assured all the tests are passed.

Soham1803 commented 6 months ago

Hey @ManyTheFish sorry for the delay. Thanks for your suggested changes. The last commit passed all the tests on local successfully, after many different tryouts. Made some major changes to the Normalizer. Suggest me any changes if necessary before the merge.

Thank You!

curquiza commented 6 months ago

Hello @Soham1803

@ManyTheFish is on Holidays and will review your PR when coming back

In the meantime, can you fix the Rustfmt tests? 😊

Thanks again for your PR

Soham1803 commented 6 months ago

Hello @curquiza, dealt with the Rustfmt tests. I guess all tests are passed. Now, just waiting for @ManyTheFish to suggest any required changes before the merge.

Thank You! 😊

meili-bors[bot] commented 5 months ago

Build succeeded:

Soham1803 commented 5 months ago

Nice to have you back @ManyTheFish. Thanks for accepting my PR and the help you provided. 😊