-
Notifications
You must be signed in to change notification settings - Fork 736
Add expected BC-breaking change warning to soundfile #906
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import os | ||
| import warnings | ||
| from typing import Optional, Tuple | ||
|
|
||
| import torch | ||
|
|
@@ -34,8 +35,26 @@ def load(filepath: str, | |
| offset: int = 0, | ||
| signalinfo: SignalInfo = None, | ||
| encodinginfo: EncodingInfo = None, | ||
| filetype: Optional[str] = None) -> Tuple[Tensor, int]: | ||
| filetype: Optional[str] = None, | ||
| *, | ||
| normalize: bool = True, | ||
| frame_offset: int = 0) -> Tuple[Tensor, int]: | ||
| r"""See torchaudio.load""" | ||
| warnings.warn( | ||
| 'The return type of `load` function in "soundfile" backend is going to be changed in 0.8.0. ' | ||
| 'Please refer to https://github.com/pytorch/audio/issues/903 for the detail.' | ||
| ) | ||
|
Comment on lines
+43
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's fine to me. next best thing to creating a new backend? |
||
|
|
||
| if not normalization: | ||
| warnings.warn( | ||
| '"normalization" argument is deprecated and removed in 0.8.0. Please use "normalize".') | ||
| elif not normalize: | ||
| normalization = normalize | ||
| if offset != 0: | ||
| warnings.warn( | ||
| '"offset" argument is deprecated and removed in 0.8.0. Please use "frame_offset".') | ||
| elif frame_offset != 0: | ||
| offset = frame_offset | ||
|
Comment on lines
+56
to
+57
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can always be done? |
||
|
|
||
| assert out is None | ||
| assert normalization | ||
|
|
@@ -82,9 +101,29 @@ def load_wav(filepath, **kwargs): | |
|
|
||
| @_mod_utils.requires_module('soundfile') | ||
| @common._impl_save | ||
| def save(filepath: str, src: Tensor, sample_rate: int, precision: int = 16, channels_first: bool = True) -> None: | ||
| def save( | ||
| filepath: str, | ||
| src: Tensor, | ||
| sample_rate: int, | ||
| precision: int = 16, | ||
| channels_first: bool = True, | ||
| *, | ||
| compression: Optional[float] = None, | ||
| ) -> None: | ||
| r"""See torchaudio.save""" | ||
|
|
||
| if precision: | ||
| warnings.warn( | ||
| '"precision" argument is depricated and removed in 0.8.0. In 0.8.0, ' | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: deprecated |
||
| 'convert the input Tensor to dtype of desired precision.' | ||
| ) | ||
|
|
||
| if compression is not None: | ||
| warnings.warn( | ||
| '"soundfile" backend\'s `save` function does not support changing compression level. ' | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you have an issue that gives more detail for this? |
||
| '`compression` argument is silently ignore.' | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: silently ignored There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will this parameter be removed? |
||
| ) | ||
|
|
||
| ch_idx, len_idx = (0, 1) if channels_first else (1, 0) | ||
|
|
||
| # check if save directory exists | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,6 +58,9 @@ def set_audio_backend(backend: Optional[str]): | |
| elif backend == 'sox_io': | ||
| module = sox_io_backend | ||
| elif backend == 'soundfile': | ||
| warnings.warn( | ||
| '"soundfile" backend is planned to change the function signatures in 0.8.0. ' | ||
| 'Please refer to https://github.com/pytorch/audio/issues/903 for the migration step.') | ||
|
Comment on lines
+61
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the real issue behind this particular comment is the return type? then it could be specialized to return type? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how about a local parameter that controls the return type in the load/save function? |
||
| module = soundfile_backend | ||
| else: | ||
| raise NotImplementedError(f'Unexpected backend "{backend}"') | ||
|
|
||
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.
what is the
*?