-
Notifications
You must be signed in to change notification settings - Fork 672
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
testing with sox only when sox is available #419
Conversation
Sorry for late reply. The test result is listed as follows: (base) audio\test>pytest . --maxfail 10000000000
================================================= test session starts =================================================
platform win32 -- Python 3.7.4, pytest-5.2.1, py-1.8.0, pluggy-0.13.0
plugins: arraydiff-0.3, doctestplus-0.4.0, openfiles-0.4.0, remotedata-0.3.2
collected 75 items / 1 errors / 74 selected
======================================================= ERRORS ========================================================
______________________________________ ERROR collecting test/test_functional.py _______________________________________
test_functional.py:35: in <module>
class TestFunctional(unittest.TestCase):
test_functional.py:44: in TestFunctional
waveform_train, sr_train = torchaudio.load(test_filepath)
..\torchaudio\__init__.py:86: in load
filetype=filetype,
..\torchaudio\_soundfile_backend.py:86: in load
filepath, frames=num_frames, start=offset, dtype="float32", always_2d=True
\Anaconda3\lib\site-packages\soundfile.py:257: in read
subtype, endian, format, closefd) as f:
\Anaconda3\lib\site-packages\soundfile.py:629: in __init__
self._file = self._open(file, mode_int, closefd)
\Anaconda3\lib\site-packages\soundfile.py:1184: in _open
"Error opening {0!r}: ".format(self.name))
\Anaconda3\lib\site-packages\soundfile.py:1357: in _error_check
raise RuntimeError(prefix + _ffi.string(err_str).decode('utf-8', 'replace'))
E RuntimeError: Error opening '\\AppData\\Local\\Temp\\tmp1nhtbbu0\\assets\\steam-train-whistle-daniel_simon.mp3': File contains data in an unknown format.
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 errors during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
================================================== 1 error in 11.87s ================================================== |
a22ddc3
to
2f470ed
Compare
Updated test results with PyTorch 1.3.1: https://gist.github.com/peterjc123/02f6cb8a00d8cdbc69b9adeb14a7c3ca |
Thanks, I've updated the PR to get rid of more RuntimeError when sox is absent. |
This seems like a regression in test coverage due to an OS specific need? Ideally we could detect the available backends at runtime. |
All the tests that were running before this PR will still be running after. Currently, the CI on Windows is not active precisely because Windows only has SoundFile, and all the tests depending on SoX would fail. (Some tests now use the
The current mechanic to determine which backend is available is in |
dad9c61
to
470e655
Compare
@peterjc123 -- The windows build is failing, have you encountered this issue before? Could you also tell me if this new version of this PR reduced the number of failing tests in windows? |
Linking #466 for general awareness |
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.
LGTM.
Made some comments for readability improvement.
test/_test.py
Outdated
torchaudio.set_audio_backend(previous_backend) | ||
|
||
|
||
def get_backends_with_mp3(backends): |
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.
The purpose of this function was not immediately clear to me at the first time reading it. Adding docstring would be helpful.
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 for pointing this out!
test/test.py
Outdated
def __exit__(self, type, value, traceback): | ||
backend = self.previous_backend | ||
torchaudio.set_audio_backend(backend) | ||
from _test import AudioBackendScope, BACKENDS, BACKENDS_MP3 |
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.
The name of the module, _test
, is not very descriptive.
Also, to me, prefixing with underscore does not give additional context, as test modules are never intended for external use. So, something like backend_utils
sound more appropriate.
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.
Good point. In fact, I'll just move those functions in common_utils.py
:)
c601ec7
to
cde66ff
Compare
Address RuntimeError in Windows CI tests that occur due to sox not being present, see here.
@peterjc123 -- please let me know if this indeed reduces the number of errors in windows.