Fix update bug #442
Fix update bug #442
Conversation
|
Hello @wannaphong! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2020-06-27 05:23:25 UTC |
|
อ๋อ ผมนึกออกละ ตัว "file" นี่คือมันเป็นชื่อ field จากรุ่นเก่าใช่ไหมครับ |
|
What about a separated script that the user will run only once to update their corpus database? |
Yes. |
|
btw, should we choose only one between "file_name" and "filename" ? now we have "file_name" in the local db and "filename" in db.json |
I think we should change it according to |
|
I think if we can find a way to update |
|
I updated |
| print("Update Corpus...") | ||
| local_db = TinyDB(corpus_db_path()) | ||
| item_all = local_db.all() | ||
| local_db.close() |
There was a problem hiding this comment.
I think it is better to use with to handle IO related to variable. To illustrate, if L100 is failed, local_db might not release the TinyDB resource. This is because the close command isn't executed because the program is failed at L100.
In this case, we should use
with TinyDB(...) as local_db:
# do something.
See detailed explanation: https://stackoverflow.com/a/2738468.
pythainlp/corpus/core.py
Outdated
| # check if the corpus is in local catalog, download if not | ||
| corpus_db_detail = get_corpus_db_detail(name) | ||
| if not corpus_db_detail or not corpus_db_detail.get("file_name"): | ||
| if (corpus_db_detail.get("file_name") is not None or corpus_db_detail.get("file") is not None) and corpus_db_detail.get("filename") is None: |
There was a problem hiding this comment.
This condition is quite hard to digest.
|
@heytitle OK. I Fixed it. |
We encountered a problem when updating to the new version. The old corpus database does not work with the new version. This pull request will fix the new version update problem.