Skip to content

Add emoji convert #512

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Jan 2, 2021
Merged

Add emoji convert #512

merged 13 commits into from
Jan 2, 2021

Conversation

@ppirch
Copy link
Contributor

@ppirch ppirch commented Dec 30, 2020 •
edited by bact
Loading

What does this changes

Implement emoji convert for Thai language from https://www.emojiall.com/th/all-emojis

Will close #469

@ppirch ppirch mentioned this pull request Dec 30, 2020
Closed
@ppirch ppirch closed this Dec 30, 2020
@wannaphong
Copy link
Member

Why you close this pull request?

@ppirch
Copy link
Contributor Author

ppirch commented Dec 30, 2020

Why you close this pull request?

I forgot import function in unit test

@ppirch ppirch reopened this Dec 30, 2020
@coveralls
Copy link

coveralls commented Dec 30, 2020 •
edited
Loading

Coverage Status

Coverage increased (+0.01%) to 95.857% when pulling a4f2952 on ppirch:add-emoji-convert into c2e65bc on PyThaiNLP:dev.

@ppirch
Copy link
Contributor Author

ppirch commented Dec 30, 2020

Why you close this pull request?

I forgot import function in unit test

I'm finished new commit. You can check it.

@bact bact self-requested a review December 31, 2020 04:38
@bact bact added the enhancement enhance functionalities label Dec 31, 2020
@bact bact added this to the 2.3 milestone Dec 31, 2020
@wannaphong
Copy link
Member

Google colab for test https://colab.research.google.com/drive/1rPhv0vRk1a3vCu1oWz9-ehs3FmLDbkqj?usp=sharing

wannaphong
wannaphong reviewed Dec 31, 2020
View reviewed changes
Copy link
Member

@wannaphong wannaphong left a comment

Choose a reason for hiding this comment

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

Thank you for pull request. Your code looks good. I review some code. PyThaiNLP use docstring reStructuredText style. I suggest you modify the document format according to my comments.

@pep8speaks
Copy link

pep8speaks commented Dec 31, 2020 •
edited
Loading

Hello @ppirch! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-01-01 06:35:03 UTC

@ppirch
Copy link
Contributor Author

ppirch commented Dec 31, 2020

Thank you for pull request. Your code looks good. I review some code. PyThaiNLP use docstring reStructuredText style. I suggest you modify the document format according to my comments.

Thank for reviewing my code. I update document format based on your suggestions.

@wannaphong
Copy link
Member

Happy New Year!

wannaphong
wannaphong approved these changes Dec 31, 2020
View reviewed changes
Copy link
Member

@wannaphong wannaphong left a comment

Choose a reason for hiding this comment

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

I thinks it's pass. wait @bact

@ppirch
Copy link
Contributor Author

ppirch commented Jan 1, 2021

Happy New Year!

Happy New Year 2021 ^_^

bact
bact approved these changes Jan 1, 2021
View reviewed changes
Copy link
Member

@bact bact left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. Looks good.

pythainlp/util/emojiconv.py Outdated
# output: :ธง_ไทย: นี่คิือธงประเทศไทย
"""

emoji_regex = re.compile("|".join(map(re.escape, _emojis)))
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can compile this in advanced, outside of the function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wait a sec.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It has been done.

@wannaphong wannaphong merged commit 6ac71f6 into PyThaiNLP:dev Jan 2, 2021
@wannaphong wannaphong mentioned this pull request Jan 2, 2021
Closed
@wannaphong wannaphong mentioned this pull request Apr 4, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bact bact bact approved these changes

@wannaphong wannaphong wannaphong approved these changes

Assignees

No one assigned

Labels

enhancement enhance functionalities

Projects

None yet

Milestone

2.3

Development

Successfully merging this pull request may close these issues.

Add emoji util

5 participants

@ppirch @wannaphong @coveralls @pep8speaks @bact