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

Tokenizer lexicons #14

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

HAKSOAT
Copy link
Contributor

@HAKSOAT HAKSOAT commented Jun 12, 2020

Created lexicons for cjk and non-cjk texts

@HAKSOAT HAKSOAT changed the title Created lexicons for cjk and non-cjk texts Tokenizer lexicons Jun 12, 2020
@HAKSOAT HAKSOAT force-pushed the tokenizer_scripts branch from 8e0c442 to e9bd355 Compare June 12, 2020 23:19
@@ -97,4 +72,10 @@
("etc", r"."),
]

wikitext_split = RegexTokenizer(LEXICON)
LEXICON_LATIN = LEXICON.copy()
LEXICON_LATIN.insert(-2, ('cjk', cjk))
Copy link
Owner

Choose a reason for hiding this comment

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

Why not insert this right after "word"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can do that. My thought process was since we won't have lots of cjk in a regular latin-dominant text, we don't need to handle them before the tab_open, tab_close, etc.


word = r'(?:[^\W\d]|[' + combined_word + r'])' + \
cjk_re = r'\u3040-\u30ff' + r'\u4e00-\u9FFF'
Copy link
Owner

Choose a reason for hiding this comment

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

Does this still cover the full range?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. It does.


cjk = r'[' + cjk_re + ']'

word = r'(?:[^\W\d' + cjk_re + r']|[' + combined_word + r'])' + \
Copy link
Owner

Choose a reason for hiding this comment

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

Do we need to explicitly exclude CJK here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without doing that, some cjk values get captured as word.

@HAKSOAT HAKSOAT force-pushed the tokenizer_scripts branch 6 times, most recently from f3489da to 1d59026 Compare June 22, 2020 20:02
@HAKSOAT HAKSOAT force-pushed the tokenizer_scripts branch from 1d59026 to df6225b Compare June 22, 2020 20:17
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.

2 participants