Proposed API change for some functions in scipy.signal
I've been using quite a few of the functions in scipy.signal recently, and I've been reminded of some of the quirks of the API. I have submitted a pull request to clean up one of those quirks. In the new pull request, I have added the argument 'fs' (the sampling frequency) to the following functions: firwin, firwin2, firls, and remez. For firwin, firwin2 and firls, the new argument replaces 'nyq', and for remez, it replaces 'Hz'. This makes these functions consistent with the other functions in which the sampling frequency is specified using 'fs': periodogram, welch, csd, coherence, spectrogram, stft, and istft. 'fs', or in LaTeX, $f_s$, is common notation for the sampling frequency in the DSP literature. In the pull request, the old argument is given a "soft" deprecation. That means the docstring says the argument is deprecated, but code to actually generate a DeprecationWarning has not been added yet. I'm fine with adding that now, but some might prefer a relatively long and soft transition for these changes. (Personally, I don't mind if they hang around for a while, but they should be gone by 2.0. :) I haven't changed the default value of the sampling frequency. For the FIR filter design functions firls, firwin and firwin2, the default is nyq=1.0 (equivalent to fs=2), while for remez the default is Hz=1 (i.e. fs=1). The functions that currently already use 'fs' all have the default fs=1. Changing the default for the FIR design functions would be a much more disruptive change. Comments here or on the pull request are welcome. P.S. I can see future pull requests in which 'fs' is added to functions that currently don't have an argument to specify the sampling frequency. I'm looking at you, freqz.
Hi Warren, Good to see that you are working on improving scipy.signal API. I have not strong opinion on the changes that you propose. I am really not an expert in DSP. I do find scipy.signal difficult to work with. While we are at it: is there a reason that scipy.signal.spline_filter only support 2D arrays? I naively thought that it would be good functionality to have on 1D signals. Gaël On Thu, Aug 31, 2017 at 01:22:26AM -0400, Warren Weckesser wrote:
I've been using quite a few of the functions in scipy.signal recently, and I've been reminded of some of the quirks of the API. I have submitted a pull request to clean up one of those quirks.
In the new pull request, I have added the argument 'fs' (the sampling frequency) to the following functions: firwin, firwin2, firls, and remez. For firwin, firwin2 and firls, the new argument replaces 'nyq', and for remez, it replaces 'Hz'. This makes these functions consistent with the other functions in which the sampling frequency is specified using 'fs': periodogram, welch, csd, coherence, spectrogram, stft, and istft. 'fs', or in LaTeX, $f_s$, is common notation for the sampling frequency in the DSP literature.
In the pull request, the old argument is given a "soft" deprecation. That means the docstring says the argument is deprecated, but code to actually generate a DeprecationWarning has not been added yet. I'm fine with adding that now, but some might prefer a relatively long and soft transition for these changes. (Personally, I don't mind if they hang around for a while, but they should be gone by 2.0. :)
I haven't changed the default value of the sampling frequency. For the FIR filter design functions firls, firwin and firwin2, the default is nyq=1.0 (equivalent to fs=2), while for remez the default is Hz=1 (i.e. fs=1). The functions that currently already use 'fs' all have the default fs=1. Changing the default for the FIR design functions would be a much more disruptive change.
Comments here or on the pull request are welcome.
P.S. I can see future pull requests in which 'fs' is added to functions that currently don't have an argument to specify the sampling frequency. I'm looking at you, freqz.
_______________________________________________ SciPy-Dev mailing list SciPy-Dev@python.org https://mail.python.org/mailman/listinfo/scipy-dev
-- Gael Varoquaux Researcher, INRIA Parietal NeuroSpin/CEA Saclay , Bat 145, 91191 Gif-sur-Yvette France Phone: ++ 33-1-69-08-79-68 http://gael-varoquaux.info http://twitter.com/GaelVaroquaux
On Thu, Aug 31, 2017 at 6:13 AM, Gael Varoquaux < gael.varoquaux@normalesup.org> wrote:
Hi Warren,
Good to see that you are working on improving scipy.signal API. I have not strong opinion on the changes that you propose. I am really not an expert in DSP. I do find scipy.signal difficult to work with.
While we are at it: is there a reason that scipy.signal.spline_filter only support 2D arrays? I naively thought that it would be good functionality to have on 1D signals.
Sorry, I don't know the reason for that. Warren Gaël
On Thu, Aug 31, 2017 at 01:22:26AM -0400, Warren Weckesser wrote:
I've been using quite a few of the functions in scipy.signal recently, and I've been reminded of some of the quirks of the API. I have submitted a pull request to clean up one of those quirks.
In the new pull request, I have added the argument 'fs' (the sampling frequency) to the following functions: firwin, firwin2, firls, and remez. For firwin, firwin2 and firls, the new argument replaces 'nyq', and for remez, it replaces 'Hz'. This makes these functions consistent with the other functions in which the sampling frequency is specified using 'fs': periodogram, welch, csd, coherence, spectrogram, stft, and istft. 'fs', or in LaTeX, $f_s$, is common notation for the sampling frequency in the DSP literature.
In the pull request, the old argument is given a "soft" deprecation. That means the docstring says the argument is deprecated, but code to actually generate a DeprecationWarning has not been added yet. I'm fine with adding that now, but some might prefer a relatively long and soft transition for these changes. (Personally, I don't mind if they hang around for a while, but they should be gone by 2.0. :)
I haven't changed the default value of the sampling frequency. For the FIR filter design functions firls, firwin and firwin2, the default is nyq=1.0 (equivalent to fs=2), while for remez the default is Hz=1 (i.e. fs=1). The functions that currently already use 'fs' all have the default fs=1. Changing the default for the FIR design functions would be a much more disruptive change.
Comments here or on the pull request are welcome.
P.S. I can see future pull requests in which 'fs' is added to functions that currently don't have an argument to specify the sampling frequency. I'm looking at you, freqz.
_______________________________________________ SciPy-Dev mailing list SciPy-Dev@python.org https://mail.python.org/mailman/listinfo/scipy-dev
-- Gael Varoquaux Researcher, INRIA Parietal NeuroSpin/CEA Saclay , Bat 145, 91191 Gif-sur-Yvette France Phone: ++ 33-1-69-08-79-68 http://gael-varoquaux.info http://twitter.com/GaelVaroquaux _______________________________________________ SciPy-Dev mailing list SciPy-Dev@python.org https://mail.python.org/mailman/listinfo/scipy-dev
This is an excellent change! What does the code do if the caller specifies both `fs` and `nyq`? On Wed, Aug 30, 2017 at 10:22 PM, Warren Weckesser < warren.weckesser@gmail.com> wrote:
I've been using quite a few of the functions in scipy.signal recently, and I've been reminded of some of the quirks of the API. I have submitted a pull request to clean up one of those quirks.
In the new pull request, I have added the argument 'fs' (the sampling frequency) to the following functions: firwin, firwin2, firls, and remez. For firwin, firwin2 and firls, the new argument replaces 'nyq', and for remez, it replaces 'Hz'. This makes these functions consistent with the other functions in which the sampling frequency is specified using 'fs': periodogram, welch, csd, coherence, spectrogram, stft, and istft. 'fs', or in LaTeX, $f_s$, is common notation for the sampling frequency in the DSP literature.
In the pull request, the old argument is given a "soft" deprecation. That means the docstring says the argument is deprecated, but code to actually generate a DeprecationWarning has not been added yet. I'm fine with adding that now, but some might prefer a relatively long and soft transition for these changes. (Personally, I don't mind if they hang around for a while, but they should be gone by 2.0. :)
I haven't changed the default value of the sampling frequency. For the FIR filter design functions firls, firwin and firwin2, the default is nyq=1.0 (equivalent to fs=2), while for remez the default is Hz=1 (i.e. fs=1). The functions that currently already use 'fs' all have the default fs=1. Changing the default for the FIR design functions would be a much more disruptive change.
Comments here or on the pull request are welcome.
P.S. I can see future pull requests in which 'fs' is added to functions that currently don't have an argument to specify the sampling frequency. I'm looking at you, freqz.
_______________________________________________ SciPy-Dev mailing list SciPy-Dev@python.org https://mail.python.org/mailman/listinfo/scipy-dev
It currently is set to throw an error <https://github.com/scipy/scipy/pull/7814>. Eric On Thu, Aug 31, 2017 at 2:41 PM, Phillip Feldman < phillip.m.feldman@gmail.com> wrote:
This is an excellent change!
What does the code do if the caller specifies both `fs` and `nyq`?
On Wed, Aug 30, 2017 at 10:22 PM, Warren Weckesser < warren.weckesser@gmail.com> wrote:
I've been using quite a few of the functions in scipy.signal recently, and I've been reminded of some of the quirks of the API. I have submitted a pull request to clean up one of those quirks.
In the new pull request, I have added the argument 'fs' (the sampling frequency) to the following functions: firwin, firwin2, firls, and remez. For firwin, firwin2 and firls, the new argument replaces 'nyq', and for remez, it replaces 'Hz'. This makes these functions consistent with the other functions in which the sampling frequency is specified using 'fs': periodogram, welch, csd, coherence, spectrogram, stft, and istft. 'fs', or in LaTeX, $f_s$, is common notation for the sampling frequency in the DSP literature.
In the pull request, the old argument is given a "soft" deprecation. That means the docstring says the argument is deprecated, but code to actually generate a DeprecationWarning has not been added yet. I'm fine with adding that now, but some might prefer a relatively long and soft transition for these changes. (Personally, I don't mind if they hang around for a while, but they should be gone by 2.0. :)
I haven't changed the default value of the sampling frequency. For the FIR filter design functions firls, firwin and firwin2, the default is nyq=1.0 (equivalent to fs=2), while for remez the default is Hz=1 (i.e. fs=1). The functions that currently already use 'fs' all have the default fs=1. Changing the default for the FIR design functions would be a much more disruptive change.
Comments here or on the pull request are welcome.
P.S. I can see future pull requests in which 'fs' is added to functions that currently don't have an argument to specify the sampling frequency. I'm looking at you, freqz.
_______________________________________________ SciPy-Dev mailing list SciPy-Dev@python.org https://mail.python.org/mailman/listinfo/scipy-dev
_______________________________________________ SciPy-Dev mailing list SciPy-Dev@python.org https://mail.python.org/mailman/listinfo/scipy-dev
On Thu, Aug 31, 2017 at 2:41 PM, Phillip Feldman < phillip.m.feldman@gmail.com> wrote:
This is an excellent change!
Thanks Phillip. What does the code do if the caller specifies both `fs` and `nyq`?
It raises an exception: ValueError("Values cannot be given for both 'nyq' and 'fs'.") Warren On Wed, Aug 30, 2017 at 10:22 PM, Warren Weckesser <
warren.weckesser@gmail.com> wrote:
I've been using quite a few of the functions in scipy.signal recently, and I've been reminded of some of the quirks of the API. I have submitted a pull request to clean up one of those quirks.
In the new pull request, I have added the argument 'fs' (the sampling frequency) to the following functions: firwin, firwin2, firls, and remez. For firwin, firwin2 and firls, the new argument replaces 'nyq', and for remez, it replaces 'Hz'. This makes these functions consistent with the other functions in which the sampling frequency is specified using 'fs': periodogram, welch, csd, coherence, spectrogram, stft, and istft. 'fs', or in LaTeX, $f_s$, is common notation for the sampling frequency in the DSP literature.
In the pull request, the old argument is given a "soft" deprecation. That means the docstring says the argument is deprecated, but code to actually generate a DeprecationWarning has not been added yet. I'm fine with adding that now, but some might prefer a relatively long and soft transition for these changes. (Personally, I don't mind if they hang around for a while, but they should be gone by 2.0. :)
I haven't changed the default value of the sampling frequency. For the FIR filter design functions firls, firwin and firwin2, the default is nyq=1.0 (equivalent to fs=2), while for remez the default is Hz=1 (i.e. fs=1). The functions that currently already use 'fs' all have the default fs=1. Changing the default for the FIR design functions would be a much more disruptive change.
Comments here or on the pull request are welcome.
P.S. I can see future pull requests in which 'fs' is added to functions that currently don't have an argument to specify the sampling frequency. I'm looking at you, freqz.
_______________________________________________ SciPy-Dev mailing list SciPy-Dev@python.org https://mail.python.org/mailman/listinfo/scipy-dev
_______________________________________________ SciPy-Dev mailing list SciPy-Dev@python.org https://mail.python.org/mailman/listinfo/scipy-dev
On Thu, Aug 31, 2017 at 5:22 PM, Warren Weckesser < warren.weckesser@gmail.com> wrote:
I've been using quite a few of the functions in scipy.signal recently, and I've been reminded of some of the quirks of the API. I have submitted a pull request to clean up one of those quirks.
In the new pull request, I have added the argument 'fs' (the sampling frequency) to the following functions: firwin, firwin2, firls, and remez. For firwin, firwin2 and firls, the new argument replaces 'nyq', and for remez, it replaces 'Hz'. This makes these functions consistent with the other functions in which the sampling frequency is specified using 'fs': periodogram, welch, csd, coherence, spectrogram, stft, and istft. 'fs', or in LaTeX, $f_s$, is common notation for the sampling frequency in the DSP literature.
In the pull request, the old argument is given a "soft" deprecation. That means the docstring says the argument is deprecated, but code to actually generate a DeprecationWarning has not been added yet. I'm fine with adding that now, but some might prefer a relatively long and soft transition for these changes. (Personally, I don't mind if they hang around for a while, but they should be gone by 2.0. :)
Good idea, +1 for a soft deprecation. Ralf
I haven't changed the default value of the sampling frequency. For the FIR filter design functions firls, firwin and firwin2, the default is nyq=1.0 (equivalent to fs=2), while for remez the default is Hz=1 (i.e. fs=1). The functions that currently already use 'fs' all have the default fs=1. Changing the default for the FIR design functions would be a much more disruptive change.
Comments here or on the pull request are welcome.
P.S. I can see future pull requests in which 'fs' is added to functions that currently don't have an argument to specify the sampling frequency. I'm looking at you, freqz.
_______________________________________________ SciPy-Dev mailing list SciPy-Dev@python.org https://mail.python.org/mailman/listinfo/scipy-dev
participants (5)
-
Eric Larson -
Gael Varoquaux -
Phillip Feldman -
Ralf Gommers -
Warren Weckesser