Update documentation and fix docstrings #788
Conversation
mthrok
force-pushed
the
doc-backend
branch
2 times, most recently
from
5d4b7d9 to
f983586
Compare
Codecov Report
@@ Coverage Diff @@
## master #788 +/- ##
==========================================
- Coverage 89.89% 89.75% -0.15%
==========================================
Files 35 34 -1
Lines 2712 2655 -57
==========================================
- Hits 2438 2383 -55
+ Misses 274 272 -2
Continue to review full report at Codecov.
|
mthrok
force-pushed
the
doc-backend
branch
6 times, most recently
from
0818a1d to
28de9fb
Compare
|
|
||
| .. warning:: | ||
| Although ``sox`` backend is default for backward compatibility reason, it has a number of issues, therefore it is highly recommended to use ``sox_io`` backend instead. Note, however, that due to the interface refinement, functions defined in ``sox`` backend and those defined in ``sox_io`` backend do not have the signatures. | ||
| Although ``sox`` backend is default for backward compatibility reason, it has a number of issues, therefore it is highly recommended to use ``sox_io`` backend instead. Note, however, that due to the interface refinement, functions defined in ``sox`` backend and those defined in ``sox_io`` backend do not have the same signatures. |
There was a problem hiding this comment.
Actually, let's simplify the statement:
soxbackend is currently the default for backward compatibility reason. It is recommended to migrate tosox_iobackend, and use its new interface, which will become the default in a future version. See here for more information about how to migrate.
There was a problem hiding this comment.
It's better to do that after we figure out the plan for deprecation of sox backend.
There was a problem hiding this comment.
My point is mostly to remove the idea of "issues", "interface refinement". How about this then?
soxbackend is the default for backward compatibility reason, but it is recommended to usesox_iobackend instead. Note, however, functions defined insoxbackend and those defined insox_iobackend do not have the same signatures.
Although sox backend is default for backward compatibility reason, it has a number of issues, therefore it is highly recommended to use sox_io backend instead. Note, however, that due to the interface refinement, functions defined in sox backend and those defined in sox_io backend do not have the signatures.
There was a problem hiding this comment.
My point here is to discourage users to use sox backend and raise the awareness of the danger of using it. If we are not upfront of issues, users would not take efforts to migrate because it takes engineering efforts to do so.
There was a problem hiding this comment.
Can you elaborate on "dangers"?
There was a problem hiding this comment.
As it's summarized in #726
sox backend does not load wav files other than 16-bit signed integer, save function does not preserve data, info returns the number of samples instead of the number of frames. This affects every aspect of ML application, from training to inference. There is always chance to introduce wrong data when using load and if one works on generative models, save function does not correctly save the data.
Ideally we should fix it but the inconsistent design and band aides placed in a wrong place to fix the issue(load_wav), it's very difficult to do that now without breaking BC.
There was a problem hiding this comment.
Thanks for the clarification. I understand what you mean though I don't think this is the correct wording. Since I do not want to block this change here for this, I'll approve the pull request.
|
|
||
|
|
||
| def set_audio_backend(backend: Optional[str]) -> None: | ||
| def set_audio_backend(backend: Optional[str]): |
There was a problem hiding this comment.
nit: why are you removing the typing?
There was a problem hiding this comment.
This typing is redundant and messed up the documentation rendering.
|
thanks! |
- Addresses pytorch#549 pytorch#638 pytorch#786 - Add `torchaudio` top level module doc - Separate `torchaudio` top level module doc from `index.html` - Add `backend` module doc. - Remove `-> None` from function signature as it adds noise to documentation - Changed function argument name of `torchaudio.backend.sox_io_backend.save` from `tensor` to `src`, so that it matches with the reset of backends. - Tweak bunch of docstrings
* Update documentation and fix docstrings (#788) - Addresses #549 #638 #786 - Add `torchaudio` top level module doc - Separate `torchaudio` top level module doc from `index.html` - Add `backend` module doc. - Remove `-> None` from function signature as it adds noise to documentation - Changed function argument name of `torchaudio.backend.sox_io_backend.save` from `tensor` to `src`, so that it matches with the reset of backends. - Tweak bunch of docstrings * addressing feedback. Co-authored-by: moto <[email protected]>
2 participants
torchaudiotop level module doctorchaudiotop level module doc fromindex.htmlbackendmodule doc.-> Nonefrom function signature as it adds noise to documentationtorchaudio.backend.sox_io_backend.savefromtensortosrc, so that it matches with the reset of backends.