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

Implemented and tested CLI for multi-language support. #60

Merged
merged 1 commit into from
Apr 16, 2018

Conversation

alecastle
Copy link

To provide support for multiple languages (issue #37), I implemented a command-line interface with the following options (described neatly in click's built in help messaging system):

image

I also added a test module to test the command-line interface. There weren't that many things I could find a way to assert, so it still requires some amount of manually checking the output, but it's definitely a lot faster than jumping around directories and running the commands separately.

Implementing this fix unfortunately required introducing an external dependency on the click library to an otherwise fairly self-contained program. However, I noticed there were a couple of other dependencies already listed in requirements.txt. I went ahead and copied those into a setup.py file and changed the directory structure a bit to form a proper python package; this was mostly for my own convenience in terms of keeping dependencies resolved, simplifying the testing process, etc. I also wrote short install and run scripts to cut down on copy-paste time during development; I can't say for sure how well they would work in a production build.

When testing support for languages other than English and Arabic, I discovered that one of the sites the original program downloaded from, subscene.com, has a flaw in their search algorithm: you have to search the exact title of the movie or it won't have any results (for example, "The Room" works, but "TheRoom" doesn't). Rather than trying to navigate the natural language processing nightmare that would be trying to figure out the exact title of a movie based on the filename (or other more convoluted means such as looking for it in the video itself), I decided to drop that website from the search (I did leave that section of code in for maintainability; I just never call it).

I apologize if my approach to fixing this issue is a bit "rough around the edges"; I did this as part of a class assignment that only gave us two weeks to finish (and I started the assignment late). I will have some free time over the summer, during which I could potentially interact more with the development community, but for now this is all I have to offer.

Incidentally, if you do decide to accept my patch, I would very much appreciate it if you do so as soon as possible (I get extra credit if the patch is accepted). Of course, it is entirely up to you, the project maintainers, whether to accept it based solely on its inherent merit (or lack thereof), but I thought it was worth mentioning (at least for my sake -- I am fully aware of how unlikely a quick merge is).

TL;DR Just try the program out and see if you like the new changes. You may have to modify a couple of file paths here and there, but your script was designed that way to begin with, so this shouldn't be too much of an issue. I've only tested it on Linux via command-line; I'm curious to see if it works on Mac.

To provide support for multiple languages (issue manojmj92#37), I implemented
a command line interface with several options related to languages,
including an option to list all available languages and an option to
automatically download subtitles for all available languages.
@manojmj92 manojmj92 merged commit a39866a into manojmj92:master Apr 16, 2018
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.

2 participants