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.

Sign up for GitHub

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

Jump to bottom

DEP: signal: remove nyq / Hz kwargs in firwin* and switch to kwarg-only #20578

Merged
merged 1 commit into from Apr 29, 2024
+11 −122
Merged

DEP: signal: remove nyq / Hz kwargs in firwin* and switch to kwarg-only #20578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
78 changes: 11 additions & 67 deletions scipy/signal/_fir_filter_design.py
View file
Open in desktop
Expand Up @@ -10,7 +10,6 @@
from scipy.special import sinc
from scipy.linalg import (toeplitz, hankel, solve, LinAlgError, LinAlgWarning,
lstsq)
from scipy._lib.deprecation import _NoValue, _deprecate_positional_args
from scipy.signal._arraytools import _validate_fs

from . import _sigtools
Expand All @@ -19,25 +18,6 @@
'firwin', 'firwin2', 'remez', 'firls', 'minimum_phase']


def _get_fs(fs, nyq):
"""
Utility for replacing the argument 'nyq' (with default 1) with 'fs'.
"""
if nyq is _NoValue and fs is None:
fs = 2
elif nyq is not _NoValue:
if fs is not None:
raise ValueError("Values cannot be given for both 'nyq' and 'fs'.")
msg = ("Keyword argument 'nyq' is deprecated in favour of 'fs' and "
"will be removed in SciPy 1.14.0.")
warnings.warn(msg, DeprecationWarning, stacklevel=3)
if nyq is None:
fs = 2
else:
fs = 2*nyq
return fs


# Some notes on function parameters:
#
# `cutoff` and `width` are given as numbers between 0 and 1. These are
Expand Down Expand Up @@ -268,9 +248,8 @@ def kaiserord(ripple, width):
return int(ceil(numtaps)), beta


@_deprecate_positional_args(version="1.14")
def firwin(numtaps, cutoff, *, width=None, window='hamming', pass_zero=True,
scale=True, nyq=_NoValue, fs=None):
scale=True, fs=None):
"""
FIR filter design using the window method.

Expand Down Expand Up @@ -320,13 +299,6 @@ def firwin(numtaps, cutoff, *, width=None, window='hamming', pass_zero=True,
`fs/2` (i.e the filter is a single band highpass filter);
center of first passband otherwise

nyq : float, optional, deprecated
This is the Nyquist frequency. Each frequency in `cutoff` must be
between 0 and `nyq`. Default is 1.

.. deprecated:: 1.0.0
`firwin` keyword argument `nyq` is deprecated in favour of `fs` and
will be removed in SciPy 1.14.0.
fs : float, optional
The sampling frequency of the signal. Each frequency in `cutoff`
must be between 0 and ``fs/2``. Default is 2.
Expand Down Expand Up @@ -397,8 +369,9 @@ def firwin(numtaps, cutoff, *, width=None, window='hamming', pass_zero=True,
# The major enhancements to this function added in November 2010 were
# developed by Tom Krauss (see ticket #902).
fs = _validate_fs(fs, allow_none=True)
fs = 2 if fs is None else fs

nyq = 0.5 * _get_fs(fs, nyq)
nyq = 0.5 * fs

cutoff = np.atleast_1d(cutoff) / float(nyq)

Expand Down Expand Up @@ -493,8 +466,7 @@ def firwin(numtaps, cutoff, *, width=None, window='hamming', pass_zero=True,
# Original version of firwin2 from scipy ticket #457, submitted by "tash".
#
# Rewritten by Warren Weckesser, 2010.
@_deprecate_positional_args(version="1.14")
def firwin2(numtaps, freq, gain, *, nfreqs=None, window='hamming', nyq=_NoValue,
def firwin2(numtaps, freq, gain, *, nfreqs=None, window='hamming',
antisymmetric=False, fs=None):
"""
FIR filter design using the window method.
Expand Down Expand Up @@ -529,13 +501,6 @@ def firwin2(numtaps, freq, gain, *, nfreqs=None, window='hamming', nyq=_NoValue,
Window function to use. Default is "hamming". See
`scipy.signal.get_window` for the complete list of possible values.
If None, no window function is applied.
nyq : float, optional, deprecated
This is the Nyquist frequency. Each frequency in `freq` must be
between 0 and `nyq`. Default is 1.

.. deprecated:: 1.0.0
`firwin2` keyword argument `nyq` is deprecated in favour of `fs` and
will be removed in SciPy 1.14.0.
antisymmetric : bool, optional
Whether resulting impulse response is symmetric/antisymmetric.
See Notes for more details.
Expand Down Expand Up @@ -603,7 +568,8 @@ def firwin2(numtaps, freq, gain, *, nfreqs=None, window='hamming', nyq=_NoValue,

"""
fs = _validate_fs(fs, allow_none=True)
nyq = 0.5 * _get_fs(fs, nyq)
fs = 2 if fs is None else fs
nyq = 0.5 * fs

if len(freq) != len(gain):
raise ValueError('freq and gain must be of same length.')
Expand Down Expand Up @@ -697,8 +663,7 @@ def firwin2(numtaps, freq, gain, *, nfreqs=None, window='hamming', nyq=_NoValue,
return out


@_deprecate_positional_args(version="1.14")
def remez(numtaps, bands, desired, *, weight=None, Hz=_NoValue, type='bandpass',
def remez(numtaps, bands, desired, *, weight=None, type='bandpass',
maxiter=25, grid_density=16, fs=None):
"""
Calculate the minimax optimal filter using the Remez exchange algorithm.
Expand All @@ -723,12 +688,6 @@ def remez(numtaps, bands, desired, *, weight=None, Hz=_NoValue, type='bandpass',
weight : array_like, optional
A relative weighting to give to each band region. The length of
`weight` has to be half the length of `bands`.
Hz : scalar, optional, deprecated
The sampling frequency in Hz. Default is 1.

.. deprecated:: 1.0.0
`remez` keyword argument `Hz` is deprecated in favour of `fs` and
will be removed in SciPy 1.14.0.
type : {'bandpass', 'differentiator', 'hilbert'}, optional
The type of filter:

Expand Down Expand Up @@ -857,15 +816,7 @@ def remez(numtaps, bands, desired, *, weight=None, Hz=_NoValue, type='bandpass',

"""
fs = _validate_fs(fs, allow_none=True)
if Hz is _NoValue and fs is None:
fs = 1.0
elif Hz is not _NoValue:
if fs is not None:
raise ValueError("Values cannot be given for both 'Hz' and 'fs'.")
msg = ("'remez' keyword argument 'Hz' is deprecated in favour of 'fs'"
" and will be removed in SciPy 1.14.0.")
warnings.warn(msg, DeprecationWarning, stacklevel=2)
fs = Hz
fs = 1.0 if fs is None else fs

# Convert type
try:
Expand All @@ -883,8 +834,7 @@ def remez(numtaps, bands, desired, *, weight=None, Hz=_NoValue, type='bandpass',
maxiter, grid_density)


@_deprecate_positional_args(version="1.14")
def firls(numtaps, bands, desired, *, weight=None, nyq=_NoValue, fs=None):
def firls(numtaps, bands, desired, *, weight=None, fs=None):
"""
FIR filter design using least-squares error minimization.

Expand Down Expand Up @@ -914,13 +864,6 @@ def firls(numtaps, bands, desired, *, weight=None, nyq=_NoValue, fs=None):
A relative weighting to give to each band region when solving
the least squares problem. `weight` has to be half the size of
`bands`.
nyq : float, optional, deprecated
This is the Nyquist frequency. Each frequency in `bands` must be
between 0 and `nyq` (inclusive). Default is 1.

.. deprecated:: 1.0.0
`firls` keyword argument `nyq` is deprecated in favour of `fs` and
will be removed in SciPy 1.14.0.
fs : float, optional
The sampling frequency of the signal. Each frequency in `bands`
must be between 0 and ``fs/2`` (inclusive). Default is 2.
Expand Down Expand Up @@ -1002,7 +945,8 @@ def firls(numtaps, bands, desired, *, weight=None, nyq=_NoValue, fs=None):

"""
fs = _validate_fs(fs, allow_none=True)
nyq = 0.5 * _get_fs(fs, nyq)
fs = 2 if fs is None else fs
nyq = 0.5 * fs

numtaps = int(numtaps)
if numtaps % 2 == 0 or numtaps < 1:
Expand Down
55 changes: 0 additions & 55 deletions scipy/signal/tests/test_fir_filter_design.py
View file
Open in desktop
Expand Up @@ -234,11 +234,6 @@ def test_fs_nyq(self):
freqs, response = freqz(taps, worN=np.pi*freq_samples/nyquist)
assert_array_almost_equal(np.abs(response),
[0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0], decimal=5)
with np.testing.suppress_warnings() as sup:
sup.filter(DeprecationWarning, "Keyword argument 'nyq'")
taps2 = firwin(ntaps, cutoff=[300, 700], window=('kaiser', beta),
pass_zero=False, scale=False, nyq=nyquist)
assert_allclose(taps2, taps)

def test_bad_cutoff(self):
"""Test that invalid cutoff argument raises ValueError."""
Expand All @@ -256,10 +251,6 @@ def test_bad_cutoff(self):
# 2D array not allowed.
assert_raises(ValueError, firwin, 99, [[0.1, 0.2],[0.3, 0.4]])
# cutoff values must be less than nyq.
with np.testing.suppress_warnings() as sup:
sup.filter(DeprecationWarning, "Keyword argument 'nyq'")
assert_raises(ValueError, firwin, 99, 50.0, nyq=40)
assert_raises(ValueError, firwin, 99, [10, 20, 30], nyq=25)
assert_raises(ValueError, firwin, 99, 50.0, fs=80)
assert_raises(ValueError, firwin, 99, [10, 20, 30], fs=50)

Expand All @@ -282,12 +273,6 @@ def test_bad_pass_zero(self):
with assert_raises(ValueError, match='must have at least two'):
firwin(41, [0.5], pass_zero=pass_zero)

def test_firwin_deprecations(self):
with pytest.deprecated_call(match="argument 'nyq' is deprecated"):
firwin(1, 1, nyq=10)
with pytest.deprecated_call(match="use keyword arguments"):
firwin(58, 0.1, 0.03)

def test_fs_validation(self):
with pytest.raises(ValueError, match="Sampling.*single scalar"):
firwin2(51, .5, 1, fs=np.array([10, 20]))
Expand Down Expand Up @@ -427,10 +412,6 @@ def test_fs_nyq(self):
taps1 = firwin2(80, [0.0, 0.5, 1.0], [1.0, 1.0, 0.0])
taps2 = firwin2(80, [0.0, 30.0, 60.0], [1.0, 1.0, 0.0], fs=120.0)
assert_array_almost_equal(taps1, taps2)
with np.testing.suppress_warnings() as sup:
sup.filter(DeprecationWarning, "Keyword argument 'nyq'")
taps2 = firwin2(80, [0.0, 30.0, 60.0], [1.0, 1.0, 0.0], nyq=60.0)
assert_array_almost_equal(taps1, taps2)

def test_tuple(self):
taps1 = firwin2(150, (0.0, 0.5, 0.5, 1.0), (1.0, 1.0, 0.0, 0.0))
Expand All @@ -443,13 +424,6 @@ def test_input_modyfication(self):
firwin2(80, freq1, [1.0, 1.0, 0.0, 0.0])
assert_equal(freq1, freq2)

def test_firwin2_deprecations(self):
with pytest.deprecated_call(match="argument 'nyq' is deprecated"):
firwin2(1, [0, 10], [1, 1], nyq=10)
with pytest.deprecated_call(match="use keyword arguments"):
# from test04
firwin2(5, [0.0, 0.5, 0.5, 1.0], [1.0, 1.0, 0.0, 0.0], 8193, None)


class TestRemez:

Expand Down Expand Up @@ -491,10 +465,6 @@ def test_compare(self):
-0.003530911231040, 0.193140296954975, 0.373400753484939,
0.373400753484939, 0.193140296954975, -0.003530911231040,
-0.075943803756711, -0.041314581814658, 0.024590270518440]
with np.testing.suppress_warnings() as sup:
sup.filter(DeprecationWarning, "'remez'")
h = remez(12, [0, 0.3, 0.5, 1], [1, 0], Hz=2.)
assert_allclose(h, k)
h = remez(12, [0, 0.3, 0.5, 1], [1, 0], fs=2.)
assert_allclose(h, k)

Expand All @@ -505,18 +475,8 @@ def test_compare(self):
0.129770906801075, -0.103908158578635, 0.073641298245579,
-0.043276706138248, 0.016849978528150, 0.002879152556419,
-0.014644062687875, 0.018704846485491, -0.038976016082299]
with np.testing.suppress_warnings() as sup:
sup.filter(DeprecationWarning, "'remez'")
assert_allclose(remez(21, [0, 0.8, 0.9, 1], [0, 1], Hz=2.), h)
assert_allclose(remez(21, [0, 0.8, 0.9, 1], [0, 1], fs=2.), h)

def test_remez_deprecations(self):
with pytest.deprecated_call(match="'remez' keyword argument 'Hz'"):
remez(12, [0, 0.3, 0.5, 1], [1, 0], Hz=2.)
with pytest.deprecated_call(match="use keyword arguments"):
# from test_hilbert
remez(11, [0.1, 0.4], [1], None)

def test_fs_validation(self):
with pytest.raises(ValueError, match="Sampling.*single scalar"):
remez(11, .1, 1, fs=np.array([10, 20]))
Expand Down Expand Up @@ -607,14 +567,6 @@ def test_compare(self):
1.156090832768218]
assert_allclose(taps, known_taps)

with np.testing.suppress_warnings() as sup:
sup.filter(DeprecationWarning, "Keyword argument 'nyq'")
taps = firls(7, (0, 1, 2, 3, 4, 5), [1, 0, 0, 1, 1, 0], nyq=10)
assert_allclose(taps, known_taps)

with pytest.raises(ValueError, match='between 0 and 1'):
firls(7, [0, 1], [0, 1], nyq=0.5)

def test_rank_deficient(self):
# solve() runs but warns (only sometimes, so here we don't use match)
x = firls(21, [0, 0.1, 0.9, 1], [1, 1, 0, 0])
Expand All @@ -633,13 +585,6 @@ def test_rank_deficient(self):
assert mask.sum() > 3
assert_allclose(np.abs(h[mask]), 0., atol=1e-4)

def test_firls_deprecations(self):
with pytest.deprecated_call(match="argument 'nyq' is deprecated"):
firls(1, (0, 1), (0, 0), nyq=10)
with pytest.deprecated_call(match="use keyword arguments"):
# from test_firls
firls(11, [0, 0.1, 0.4, 0.5], [1, 1, 0, 0], None)

def test_fs_validation(self):
with pytest.raises(ValueError, match="Sampling.*single scalar"):
firls(11, .1, 1, fs=np.array([10, 20]))
Expand Down