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

Consistent naming and consolidate similar codes #146

Merged
merged 20 commits into from
Nov 4, 2018
Merged

Consistent naming and consolidate similar codes #146

merged 20 commits into from
Nov 4, 2018

Conversation

bact
Copy link
Member

@bact bact commented Nov 1, 2018

Naming

Reduce redundant codes

  • consolidate dictionary reading codes into fewer files (shared get_corpus() code)
  • remove redundant code creating new dict trie in tokenize/init.py
  • merge related code in corpus/pos.py to tag/unigram.py

Common characters set

  • pythainlp.corpus.alphabets, tone, etc becomes pythainlp.thai_alphabets, thai_tonemarks, etc. and now in a str form, which in this use case (1-char members) can iterate faster that list or set (follow the design of ascii_letters, etc. in string module)

Test cases

  • update test cases

- consolidate dictionary reading codes into fewer files (shared get_corpus() code)
- make dictionary filenames more consistent to each other (see #141)
- old new-thaidict.txt is now words_th.txt (will use as the most updated word list)
- old thaiword.txt and thaiword_frozen_201810.txt is now words_th_frozen_201810.txt (can have more versions in the future)
- pythainlp.corpus.alphabets, tone, etc becomes pythainlp.corpus.THAI_ALPHABETS, THAI_TONEMARKS, etc. and now in a ```str``` form, which in this use case (1-char members) can iterate faster that list or set (follow the design of ```ascii_letters```, etc. in ```string``` module)
- rename pythainlp/tag/old.py ->  pythainlp/tag/unigram.py
- remove redundant code creating new dict trie in tokenize/__init__.py
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 60.576% when pulling 3edda76 on bact:dev into 383051f on PyThaiNLP:dev.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 60.576% when pulling 3edda76 on bact:dev into 383051f on PyThaiNLP:dev.

@coveralls
Copy link

coveralls commented Nov 2, 2018

Coverage Status

Coverage increased (+2.2%) to 63.019% when pulling a2c5d35 on bact:dev into 383051f on PyThaiNLP:dev.

pythainlp/corpus/wordnet.py Outdated Show resolved Hide resolved
pythainlp/tools/__init__.py Show resolved Hide resolved
@wannaphong wannaphong added the enhancement enhance functionalities label Nov 2, 2018
@wannaphong wannaphong added this to the 1.8 milestone Nov 2, 2018
@bact
Copy link
Member Author

bact commented Nov 3, 2018

ผมพยายามรวมโค้ดที่ซ้ำเข้าด้วยกัน ซึ่งบางอันจะทำให้ชื่อการใช้เข้าถึงเปลี่ยนไป (ชื่อ API จะเปลี่ยน) ถ้ามีตรงไหนเปลี่ยนแล้วไม่สมเหตุผล สามารถแก้ไขได้ครับ

…wercase), as suggested by @wannaphongcom

- move them from pythainlp.corpus module to to pythainlp module since they are not really a corpus and are common variables to be shared by all modules
…/__init__.py

- reduce numbers of convenience imports in pythainlp/__init__.py to reduce namespace crashes/mutual top-level import crashes possibility
- Move isthai() function from pythainlp.tokenize to pythainlp.util
- Move wordtonum function from pythainlp.util to pythainlp.number
- Refactor codes related to pythainlp.util
- More test cases, sort test cases by import order
Copy link
Member Author

@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.

ชื่อประเทศในรายการ ให้ใส่วงเล็บภาษาไทย/ภาษาอังกฤษเข้าไปด้วยไหมครับ

pythainlp/corpus/countries_th.txt Show resolved Hide resolved
@bact bact merged commit f060442 into PyThaiNLP:dev Nov 4, 2018
@bact
Copy link
Member Author

bact commented Nov 4, 2018

เรื่องวงเล็บชื่อประเทศ ขอแยกไปเป็นอีกรอบครับ มีหลายประเทศเลย ที่มีวงเล็บ ท้ังวงเล็บไทยและวงเล็บอังกฤษ
จะเก็บไว้ไหม ขึ้นอยู่กับเราจะเอารายการนี้ไปทำอะไร

@wannaphong wannaphong mentioned this pull request Nov 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement enhance functionalities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants