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

Downloader for the Mozilla Common Voice Datasets #120

Merged
merged 1 commit into from
Apr 20, 2020
Merged

Downloader for the Mozilla Common Voice Datasets #120

merged 1 commit into from
Apr 20, 2020

Conversation

Mastercuber
Copy link

@Mastercuber Mastercuber commented Apr 8, 2020

This 3 commits add support for downloading the Common Voice datasets. All today available languages are supported. If new languages will be added, the provided tests fail or a new release of the Common Voice dataset is available then the DOWNLOAD_URLS need to be modified to include/replace the new/broken URL's.

  • Hopefully the commit message is OK
  • flake8 gives no errors
  • Docstring is added to the Downloader Class
  • Used ArchivDownloader as Base so Downloader is tested
    -> Dataset availability tests (checks if HTTP HEAD Status Code is 200) are added
  • Is straight with master

@Mastercuber
Copy link
Author

I do not understand the last DeepSource error (no self). If I remove self the tests do not run at all

@ynop
Copy link
Owner

ynop commented Apr 12, 2020

Nice, thank you.
At the moment you don't have to care about DeepSource if there is no obvious solution, I'm working on that (we have a lot of issues regarding that in the code base).

Regarding the URL availability, I think it is not good to have the tests for that in the same unit tests.
You could leave that out for now.
There is already an open issue about that (#88), but no solution so far I am really happy with.

Could you please merge the fix-commits in to the other one?

@Mastercuber
Copy link
Author

The fix commits are squashed, the "Added Test" commit is removed, DeepSource is ignored 😄 and the Test is also removed. Should look fine now.

@ynop
Copy link
Owner

ynop commented Apr 17, 2020

In the mean time I have worked through all the deepsource issues in the codebase.
Please rebase on the master, just to make sure you have not introduced new ones (which you haven't as far as i can see).

@Mastercuber
Copy link
Author

Mastercuber commented Apr 17, 2020

Rebase onto master is done, checked with flake8 and DeepSource looks good 👍

@Mastercuber
Copy link
Author

Travis says

  1. SVML not detected
  2. Failed building wheel for llvmlite
    And this for Python 3.5.
    The other Python Versions pass.

You know how to handle that problem?

@ynop
Copy link
Owner

ynop commented Apr 17, 2020

I guess has something to do with numba/llvmlite#551, they dropped py35 support. We could try to fix on an old version (It is not fixed because it is only a subdependency of librosa -> numba). But it is not the first time that llvmlite version messes up the builds. If I remember correctly it failed last time because we fixed on a specific version.

@Mastercuber
Copy link
Author

Mastercuber commented Apr 18, 2020

If audiomate will carry on support python 3.5 another library is needed if I understand it right..

Using an old version is maybe a possibility, but I sounds not right. Not supporting python 3.5 is maybe also no possibility..

@aahlenst
Copy link
Collaborator

The joy of transitive dependencies 🤢

I did not find any reasoning why llvmlite dropped Python 3.5 support. Following their lead is an option considering Python 3.5 reaches its EOL in September.

@Mastercuber
Copy link
Author

5 months left. So maybe it's a possibility to just drop the support for Python 3.5...

Otherwise, I think, it would be a bit of work to fix that.. But i just suppose that..

@ynop ynop merged commit 25eb6ee into ynop:master Apr 20, 2020
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.

3 participants