-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fix failing python tests #478
Conversation
These rely on the LAC module that has been removed because it encountered an error that prevented other tests from running
|
Makes sense to me. LAC was brought on expressly for a POC; it seemed best positioned when we were looking at it to just show that the functionality is possible, but if concerted effort was made to really get Chinese in a good, scalable and sustainable state, it would probably merit re-evaluating the library landscape. LAC doesn't pin It's not clear to me from scanning the Some relevant issues from LAC:
|
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.
Thanks @jaguillette. Disabling the lac
dependency and commenting out the chinese tests seems OK for now. As Elliott pointed out, that dependency will need to be re-evaluated anyway when we integrate it back in in along with the rest of the code that was dependent on the lattice structure.
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.
Looks good to me, and definitely agreed with what Artie and Elliott pointed out!
For some reason, the Python tests have started failing in GitHub actions. A failing run with logs can be seen here: https://github.com/Hedera-Lang-Learn/hedera/actions/runs/3603801082/jobs/6073350159. The issue seems to be with setting up
paddle
, which is being installed because oflac
, which is part of the lemmatizer for Chinese language text. Since we've removed Chinese language support from the app for the time being, this PR removeslac
from the requirements and comments out the tests that depend on it. The library and tests should be reinstated as part of bringing Chinese language support back in to the app, when that happens.