Closed RicBent closed 1 week ago
Is this change correct? https://github.com/RicBent/Kiwi/commit/3f0eb0cbe6167078680e0c6a2d6577c1ab8af08a
Passing 0 for numThreads
to prevent the ThreadPool
creation in KiwiBuilder
did not work as that triggered some assertion.
Attempted to implement the release workflow. Not sure how to test it properly without triggering a release.
I guess a workflow that triggers on new pull requests still needs to be added.
@RicBent Wow, it's amazing! Thank you for your contribution! The demo seems to work very well. I'll review it as soon as possible.
Great!
The latest commit adds a wrapper package that could be directly imported in any npm project. Along with proper types for the entirety of the so far exposed API (https://github.com/bab2min/Kiwi/blob/2161cf137b996471383e7c7b65370e9f28981f14/bindings/wasm/package/src/kiwi.ts).
I'd be happy to implement the remaining API functionality to bring it to the same level as the Python library once you had a look at the PR.
@RicBent Oh, it looks good to me. 👍 I would really appreciate it if the rest of the functionality was completed as well. If it is difficult for you to implement all the functionality, I think it is okay to implement only the core, merge and release them first, and then supplement the rest later.
I had a look at the Java bindings and it seems like the only missing API is the following:
KiwiBuilder:
Kiwi:
I would also like to add documentation to the bindings. However I am not able to make them in Korean, only in English. Is that a problem @bab2min ?
Oh and we would also need an npm package name as 'kiwi' is already taken. I chose 'kiwi-nlp' for now, but I can change it if you have a better suggestion.
@RicBent
It's okay to write the documentation in English. If you write the documentation in English, I can translate it for Korean documentation.
I think kiwi-nlp
is a good name for the package, showing that it is a NLP library.
Thank you for your contribution!
Alright, I am almost there then. I already wrote pretty much all the required documentation and just the 4 points from KiwiBuilder
are missing to match the Java API functionality.
I also amended the release workflow to automatically build and publish the resulting package to npm:
The workflow requires NPM_TOKEN
to be added to the repo's secrets on your end when this gets merged.
To generate an npm token you need to do the following:
kiwi-nlp
package)To add it to the repo's secrets you can follow this: https://docs.github.com/en/actions/security-guides/using-secrets-in-github-actions
@bab2min everything on my end should be done now:
Let me know if anything needs to be improved/changed!
@RicBent Great!!! Thank you for your contribution. I'll add documentations for Korean and tokens for release workflow.
This pull requests adds a build target for a JavaScript library, usable in any modern Browser. For this the emscripten toolchain is utilized.
Solves #169
You can check out a running demo here: https://ricbent.github.io/Kiwi/demo/
Things that can be improved:
More of the base API functionality (multiple results, custom builder arguments, etc)Addition of GitHub actions to build this new targetCreating a proper npm package (package.json, helpers like the WebWorker wrapper from the demo, TypeScript types, etc), deployed via GitHub actionsAdd DocumentationBecause I haven't set up GitHub actions for this target yet, you need to manually build. This requires the emscripten toolchain to be installed:
This will generate
kiwi-wasm.js
andkiwi-wasm.wasm
. The former can be directly used in any modern browser.Sorry if I didn't follow some contribution guidelines correctly as I don't speak much Korean (yet).
Things changed since the creation of the PR: