Skip to content
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

Merged
merged 2 commits into from
Dec 6, 2022
Merged

Fix failing python tests #478

merged 2 commits into from
Dec 6, 2022

Conversation

jaguillette
Copy link
Collaborator

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 of lac, 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 removes lac 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.

These rely on the LAC module that has been removed because it encountered an error that prevented other tests from running
@jaguillette jaguillette marked this pull request as ready for review December 2, 2022 19:48
@jaguillette
Copy link
Collaborator Author

jaguillette commented Dec 2, 2022

lac hasn't had updates since May 2021, so it isn't a recent update to that library, but one of its dependencies that my local environment picked up, paddlepaddle got an update on November 17 of this year, so maybe that did it somehow? paddlepaddle isn't in requirements/base.txt though.

@elliottyates
Copy link
Contributor

elliottyates commented Dec 2, 2022

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 paddle in any way:
https://github.com/baidu/lac/blob/master/python/setup.py

It's not clear to me from scanning the paddle releases or the LAC issues whether it's fixable, or what's causing it, but LAC certainly doesn't look maintained at the moment, judging by the Issues activity; the only issues closed since early 2021 have been by the issue creators themselves.

Some relevant issues from LAC:

Copy link
Contributor

@arthurian arthurian left a 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.

Copy link
Collaborator

@Tanvez Tanvez left a 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!

@arthurian arthurian merged commit 11cc88b into dev Dec 6, 2022
@arthurian arthurian deleted the fix-failing-python branch December 6, 2022 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants