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

Make Japanese-specific dependencies optional #2776

Merged
merged 4 commits into from
Jul 24, 2023

Conversation

polm
Copy link
Contributor

@polm polm commented Jul 17, 2023

Hello, I am the maintainer of mecab-python3, unidic-lite, and other Japanese related packages.

Your package uses my packages, but only when dealing with Japanese. My packages are unconditionally required by TTS. However, because my package involves a C++ extension, it is hard to build on M1 Macs. I also do not have the relevant hardware, and so I am unable to distribute wheels for that platform, for details see SamuraiT/mecab-python3#84.

For roughly a year (SamuraiT/mecab-python3#71), people using TTS have intermittently bothered me about how to build things on OSX or other things I am unable to help with. I explained how to fix this in December, but a member of your dev team seemed to consider it low priority, so I took it on myself to make this PR.

This change makes it so Japanese-related dependencies are not installed by default, but can be installed with tts[ja], similar to the existing tts[dev]. If someone tries to use Japanese and the required packages are missing they will get an ImportError explaining the issue. This change will hopefully make it so people stop asking me for help with my software even though they don't need it in the first place.

@CLAassistant
Copy link

CLAassistant commented Jul 17, 2023

CLA assistant check
All committers have signed the CLA.

@erogol
Copy link
Member

erogol commented Jul 17, 2023

Thanks for the PR @polm. I think you've forgotten to push the requirements file.

Also, do you plan to agree on the CLA?

@polm
Copy link
Contributor Author

polm commented Jul 17, 2023

I added the JA requirements file and agreed to the CLA.

@erogol
Copy link
Member

erogol commented Jul 17, 2023

Thanks @polm

Tests are failing since JA deps are missing in the testing environment. Probably we should install it all in testing.

This should help the tests run.
@polm
Copy link
Contributor Author

polm commented Jul 23, 2023

JA deps are installed in the test now. A few tests are still failing but they don't seem to have anything to do with Japanese and it's not clear to me why it's happening, so someone more familiar with the test sweet than I am should take a look at it.

@erogol
Copy link
Member

erogol commented Jul 24, 2023

@polm thanks I merge and deal with the rest of the CI issues.

@erogol erogol merged commit c0aabb8 into coqui-ai:dev Jul 24, 2023
34 of 40 checks passed
@gitmylo
Copy link

gitmylo commented Aug 1, 2023

When you import TTS it also tries to import the JA dependencies, even if you aren't trying to use the Japanese text to speech.

Tindell pushed a commit to pugtech-co/TTS that referenced this pull request Sep 4, 2023
* Don't install MeCab by default

* Add optional [ja] deps, like [dev] etc

* Add JA requirements file

* Add JA requirements to requirements_all

This should help the tests run.
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