-
Notifications
You must be signed in to change notification settings - Fork 286
fix the connectivity of cli commands #1154
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
base: dev
Are you sure you want to change the base?
Conversation
@bact hello, please check this PR and consider merging |
Could you add tag, benchmark, and misspell? I think the modules aren't working too. |
@wannaphong i added those also |
why is there so many failed test, can anyone guide me about how to fix this if this is my part to do |
if __name__ == "__main__": | ||
# Create a simple mapping from command name to the imported module | ||
COMMAND_MAP = { | ||
"tokenize": tokenize, | ||
"soundex": soundex, | ||
"tag": tag, | ||
"benchmark": benchmark, | ||
"misspell": misspell, | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we have all these at line 15 already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we have all these at line 15 already?
you are right @bact , but the problem is only resolved if I import those libraries in this file, but your tests shows an error stating that the libraries were never used, that is why I have to explicitly add those, if you want I can remove them .
Can you add pyyaml to https://github.com/PyThaiNLP/pythainlp/blob/dev/setup.py#L44? |
|
@wannaphong hey, i know i am doing a lot of errors, it would be great if you can approve this again one last time, i think this time everything should be fine. |
@bact , please review and consider merging |
I tested and all cli can working! https://colab.research.google.com/drive/10cEo4NC_W7alpu2eLiv_eZ4uY0SQ4a2f?usp=sharing |
What does this changes
i changes the cli init file, and now the commands
show output
BEFORE
AFTER
What was wrong
The root cause of this issue is a missing import dependency that creates a disconnect between command registration and command execution.
How this fixes it
Updated the import statement in the command line helpers module to explicitly import the tokenize and soundex submodules from pythainlp.cli. This change ensures that these modules are properly loaded and available for use by the command-line interface, preventing the no-output problem faced earlier.
Fixes #1151
Your checklist for this pull request