Closed uezo closed 4 years ago
Looks cool!
In general your patch looks good to me, thanks.
Also I think it would be great if you could add some tests for UserDictionary class in tests/test_dic.py
. There are a few tests for it already, you could add additional tests there with mock ProgressHandler
. But it's optional and can be delayed.
Seems tests on Windows fail. I'll take a look. CI pipelines on Linux are fine: https://travis-ci.org/github/mocobeta/janome/builds/722745085
@uezo about tests for UserDictionary: please add separated test methods for the indicators, instead of modifying existing test cases. Each test case should be kept as small as possible (and has only one concern) for maintainability.
Especially for Windows OS, encoding
option is mandatory. I also use context manager to ensure close opened user dict file. https://github.com/mocobeta/janome/pull/87/commits/c0b7ff427bbff1e6bca28614b88c5d671c06011d
Hi @uezo, I made a couple of fixes on the branch (by utilizing my owner privilege), please take a look. I think this is now ready to merge to upstream.
Thank you for fix! Everything looks fine! Sorry for missing to close the file...
Also I updated the documentation: https://github.com/mocobeta/janome/pull/87/commits/806d6240952d63ed8f4231a909d1eb9db50f3670
(ja)
(en)
Thank you @uezo for the nice patch!
Usage
Create instance of
SimpleProgressIndicator
and pass it as argument like below:Then, progress will be indicated.
Custom indicator
You can also use your own custom indicator. Here is the example using tqdm.